On 01/13/15 13:13, Jakub Jelinek wrote:
On Tue, Jan 13, 2015 at 12:45:27PM -0700, Jeff Law wrote:
On 01/13/15 09:38, Segher Boessenkool wrote:
On Tue, Jan 13, 2015 at 05:18:19PM +0100, Jakub Jelinek wrote:
3) on request from Richard (which Segher on IRC argues against), "memory"
clobber also prevents CSE;
As extend.texi used to say:
"
If your assembler instructions access memory in an unpredictable
fashion, add @samp{memory} to the list of clobbered registers. 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}.
"
so a "memory" clobber in a non-volatile asm should not prevent CSE.
My reading of that paragraph is somewhat different.
The key here is the memory clobber affects optimization of instructions
around the asm while the volatile specifier affects the optimization of the
ASM itself.
A memory clobber must inhibit CSE of memory references on either side of the
asm because the asm must be assumed to read or write memory in unpredictable
ways.
The volatile specifier tells the compiler that the asm itself must be
preserved, even if dataflow shows the outputs as not used.
That is not necessarily in conflict.
Possibly not :-) This stuff isn't trivial and as well meaning as folks
trying to update the docs have been, it's possible subtle issues like
these have been missed, even in the review process.
I know that for me it's easier to reason about code changes like this
than their associated documentation :-)
My reading of Jeff's comment is that in
int a;
int
foo (void)
{
int b, c, d, e;
b = a;
asm ("..." : "=r" (c) : : "memory");
d = a;
asm ("..." : "=r" (e) : : "memory");
return b + d + 2 * (c + e);
}
we are not allowed to CSE d = a; into d = b;
Precisely. At least that's how I read things and it makes sense to the
part of my brain that used to split time between kernel & GCC
development in a previous life.
In effect the "memory" clobber is an aggregation of the read, write,
clobbers dataflow for memory (and imprecise as it hits all memory).
. CSE invalidate_from_clobbers
should ensure that already, even when we don't do anything special about
"memory" clobber in the patch.
OK.
Another thing is if there is a store
in between the two non-volatile asms with "memory" clobber, here I'm not
sure if with the alternate patch we'd treat the "memory" clobber as use of
everything previously stored into memory (in this regard the posted version
is safe).
I woudln't be terribly surprised if DSE isn't safe in this regard. I
don't recall CSE doing any kind of dead store elimination so it wouldn't
likely care that the memory clobber implies a read as well.
And finally there is the case of non-volatile asm with "memory" clobber with
no memory stores in between the two - the posted (safer) patch will not
allow to CSE the two, while in theory we could CSE them into just one asm.
I think we have to assume that CSEing them is wrong. The first may set
something in memory that is read by the second.
Thoughts?
Jeff
