Re: [PATCH] Reenable CSE of non-volatile inline asm (PR rtl-optimization/63637)

[Jeff Law <law@redhat.com>]

On 01/14/15 08:19, Segher Boessenkool wrote:

> "
> @findex clobber
> @item (clobber @var{x})
> Represents the storing or possible storing of an unpredictable,
> undescribed value into @var{x}, which must be a @code{reg},
> @code{scratch}, @code{parallel} or @code{mem} expression.
>
> [...]
>
> If @var{x} is @code{(mem:BLK (const_int 0))} or
> @code{(mem:BLK (scratch))}, it means that all memory
> locations must be presumed clobbered.
> "
>
> Note it doesn't mention reading memory.
The documentation is incomplete.  The right thing to do is fix the 
documentation and treat  the "memory" tag appearing in the "clobber" 
section as a read as well as a write.

It's lame, but the historical decision by RMS to put that tag into the 
clobbers section is what it is.  Don't get too hung up on it.  RMS just 
botched it.

>
> Now if we go back to my earlier quote:
>
> "
> If your assembler instructions access memory in an unpredictable
> fashion, add @samp{memory} to the list of clobbered registers.
Note "access" not "write".


  This
> causes GCC to not keep memory values cached in registers across the
> assembler instruction and not optimize stores or loads to that memory.
> You also should add the @code{volatile} keyword if the memory
> affected is not listed in the inputs or outputs of the @code{asm}, as
> the @samp{memory} clobber does not count as a side-effect of the
> @code{asm}.
> "
>
> That last line means the compiler is free to delete a non-volatile
> asm with a memory clobber if that asm is not needed for dataflow.  Or
> that is how I read it; it is trying to indicate that if you want to
> prevent the memory clobber from being deleted (together with the rest
> of the asm), you need to make the asm volatile.
>
> So as far as I can see the compiler can CSE two identical non-volatile
> asms with memory clobber just fine.  Older GCC (I tried 4.7.2) does do
> this; current mainline doesn't.  I think it should.
No, it should not CSE those two cases.  That's simply wrong and if an 
older version did that optimization, that's a bug.

jeff