[PATCH v2] MTD: Fix Orion NAND driver compilation with ARM OABI

[Nicolas Pitre]

On Sat, 24 Apr 2010, Jamie Lokier wrote:

> Nicolas Pitre wrote:
> > On Thu, 25 Mar 2010, Jamie Lokier wrote:
> > > >  		asm volatile ("ldrd\t%0, [%1]" : "=&r" (x) : "r" (io_base));
> > > >  		buf64[i++] = x;
> > > 
> > > The "register...asm" looks fine, but it occurs to me the constraints
> > > are too weak (and they were before), so GCC could optimise that to the
> > > wrong behaviour.
> > > 
> > > The "volatile" prevents GCC deleting the asm if it's output isn't
> > > used, but it doesn't stop GCC from reordering the asms, for example if
> > > it decides to unroll the loop.  It probably won't reorder in that
> > > case, but it could.  The result would be out of order values stored
> > > into buf[].  It could even move the ldrd earlier than the prior byte
> > > accesses, or after the later byte accesses.
> > 
> > I don't see how that could happen.  The store into buf[] puts a 
> > dependency on the output constraint of the inline asm statement.  And by 
> > vertue of being volatile, gcc cannot cache the result of the output from 
> > the asm as if it was a pure function.
> 
> The store into buf[] dependency doesn't stop this, after unrolling:
> 
>     asm volatile ("ldrd\t%0, [%1]" : "=&r" (x) : "r" (io_base));
>     buf64[i++] = x;
>     asm volatile ("ldrd\t%0, [%1]" : "=&r" (x) : "r" (io_base));
>     buf64[i++] = x;
> 
> from being reordered as this
> 
>     asm volatile ("ldrd\t%0, [%1]" : "=&r" (x2) : "r" (io_base));
>     asm volatile ("ldrd\t%0, [%1]" : "=&r" (x1) : "r" (io_base));
>     buf64[i++] = x1;
>     buf64[i++] = x2;
> 
> because the asm doesn't depend on memory, just register inputs and
> outputs;

I disagree.  The volatile tells gcc that the asm has side effects, and 
therefore 1) they can't be optimized away, and 2) can't be swapped with 
regards to each other like you do in your example.

>     'Q'
>           A memory reference where the exact address is in a single
>           register (''m'' is preferable for 'asm' statements)

Hmmm... Is this something new?  I must have missed it before.


Nicolas