public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.aribaud@free.fr>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RFC PATCH v2] ARM: Avoid compiler optimization for usages of readb, writeb and friends.
Date: Tue, 21 Dec 2010 11:38:44 +0100	[thread overview]
Message-ID: <4D1083B4.2060704@free.fr> (raw)
In-Reply-To: <AANLkTikN-02Eay6u90DMKHuL8uvy7e0VLFSebxDsdoEo@mail.gmail.com>

Le 21/12/2010 09:46, John Rigby a ?crit :
> On Tue, Dec 21, 2010 at 1:35 AM, Dirk Behme<dirk.behme@googlemail.com>  wrote:
>>
>> (Resend with corrected broken example)
>>
>> On 21.12.2010 08:21, Albert ARIBAUD wrote:
>>> Hi Dirk,
>>>
>>> Le 21/12/2010 08:11, Dirk Behme a ?crit :
>>>
>>>> But the issue with drivers/mtd/nand/omap_gpmc.c (i.e. the additional
>>>> ldrb    r3, [r3]) is still open? Has anybody tried to replace it with
>>>> a nop in the binary to be sure this is the root cause?
>>>
>>> Can you try and preprocess the C file for both the broken and working
>>> cases, then post the preprocessed C extract? Differences at the C level
>>> may help understanding differences at the asm level.
>>
>> gcc version 4.5.1 (Sourcery G++ Lite 2010.09-50)
>>
>> Work:
>> ====
>>
>> static void omap_nand_hwcontrol(struct mtd_info *mtd, int32_t cmd,
>>      uint32_t ctrl)
>> {
>>   register struct nand_chip *this = mtd->priv;
>>   ...
>>   if (cmd != -1)
>>
>>    (*(volatile unsigned char *)(this->IO_ADDR_W) = (cmd));
>> }
>>
>>         if (cmd != NAND_CMD_NONE)
>>    84:  e3710001        cmn     r1, #1
>>                 origwriteb(cmd, this->IO_ADDR_W);
>>    88:  15933004        ldrne   r3, [r3, #4]
>>    8c:  120110ff        andne   r1, r1, #255    ; 0xff
>>    90:  15c31000        strbne  r1, [r3]
>>    94:  e12fff1e        bx      lr
>>         ...
>>
>>
>> Broken:
>> ======
>>
>> static void omap_nand_hwcontrol(struct mtd_info *mtd, int32_t cmd,
>>      uint32_t ctrl)
>> {
>>   register struct nand_chip *this = mtd->priv;
>>
>> ...
>>
>>   if (cmd != -1)
>>    ({ __asm__ __volatile__ ("" : : : "memory"); (*(volatile unsigned
>> char *)(this->IO_ADDR_W) = (cmd)); });
>> }
>>
>>         if (cmd != NAND_CMD_NONE)
>>    84:  e3710001        cmn     r1, #1
>>    88:  012fff1e        bxeq    lr
>>                 writeb(cmd, this->IO_ADDR_W);
>>    8c:  e5933004        ldr     r3, [r3, #4]
>>    90:  e20110ff        and     r1, r1, #255    ; 0xff
>>    94:  e5c31000        strb    r1, [r3]
>>    98:  e5d33000        ldrb    r3, [r3]
>>    9c:  e12fff1e        bx      lr
>>
>>
>> The issue seems to be the additional 'ldrb      r3, [r3]' added by the
>> compiler in the broken version.
>>
>
> And I at your suggestion tried modifying the binary changing the extra
> ldrb to a nop and it works.

Seems like a compiler issue to me, as the preprocessed C source is the 
same for the register access and does not call for a re-read (that is 
what I wanted to see in the preprocessed version), yet the ASM sequence 
does the re-read.

Amicalement,
-- 
Albert.

  reply	other threads:[~2010-12-21 10:38 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-18 22:27 [U-Boot] [RFC PATCH v2] ARM: Avoid compiler optimization for usages of readb, writeb and friends Alexander Holler
2010-12-19  7:51 ` Dirk Behme
2010-12-19 10:22   ` Alexander Holler
2010-12-19 11:28     ` Albert ARIBAUD
2010-12-19 16:34       ` Alexander Holler
2010-12-19 18:45     ` John Rigby
2010-12-19 19:59       ` Alexander Holler
2010-12-20  0:39         ` John Rigby
2010-12-20  0:56           ` Alexander Holler
2010-12-20  4:18             ` John Rigby
2010-12-20  6:07               ` John Rigby
2010-12-20  6:49                 ` Dirk Behme
2010-12-20  7:37                   ` John Rigby
2010-12-20 16:08                     ` John Rigby
2010-12-20 16:12                       ` John Rigby
2011-01-17  4:35                       ` Alexander Holler
2010-12-20 17:12                 ` Alexander Holler
2010-12-21  0:25                   ` John Rigby
2010-12-21  0:46                     ` John Rigby
2010-12-21  7:11                       ` Dirk Behme
2010-12-21  7:21                         ` Albert ARIBAUD
2010-12-21  8:05                           ` Dirk Behme
2010-12-21  8:17                             ` Wolfgang Denk
2010-12-21  8:37                               ` Dirk Behme
2010-12-21  8:35                           ` Dirk Behme
2010-12-21  8:46                             ` John Rigby
2010-12-21 10:38                               ` Albert ARIBAUD [this message]
2010-12-21 10:53                                 ` Wolfgang Denk
2010-12-21 12:35                                   ` Alexander Holler
2010-12-21 12:51                                     ` Albert ARIBAUD
2010-12-21 13:30                                       ` Alexander Holler
2010-12-21 14:33                                         ` Albert ARIBAUD
2010-12-21 19:52                                           ` Wolfgang Denk
2010-12-21 20:04                                             ` Dirk Behme
2010-12-21 21:49                                               ` Albert ARIBAUD
2010-12-22  0:11                                               ` Alexander Holler
2010-12-22  7:02                                                 ` Albert ARIBAUD
2010-12-22  7:18                                                   ` Dirk Behme
2010-12-22  7:52                                                     ` Wolfgang Denk
2010-12-23 16:40                                                       ` Dirk Behme
2010-12-22  9:56                                                   ` Alexander Holler
2010-12-21 13:38                     ` Alexander Holler
2010-12-22  8:02 ` Wolfgang Denk
2010-12-22 11:23   ` Alexander Holler
2010-12-29  9:40   ` Dirk Behme
2010-12-29 23:10     ` Alessandro Rubini
2010-12-30 10:39       ` Dirk Behme
2011-01-09 22:03       ` Wolfgang Denk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D1083B4.2060704@free.fr \
    --to=albert.aribaud@free.fr \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox