On Fri, Jan 11, 2019 at 01:53:21PM +0100, Richard Biener wrote:
> >The canary slot in the stack frame is written in the prologue using
> >MEM_VOLATILE_P store, so we never consider those to be DSEd and is only
> >read
> >in the epilogue, so it shouldn't alias any other stores.
> >Similarly, __stack_chk_guard variable or say the TLS ssp slot or
> >whatever
> >else is used to hold the random pointer-sized value really shouldn't be
> >changed in -fstack-protector* instrumented functions, as that would
> >mean
> >they remembered one value in the prologue and would fail comparison in
> >the
> >epilogue if it changed in between. So, I believe we can safely ignore
> >the
> >whole stack_pointer_test instruction in RTL DSE.
> >
> >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Isn't it enough to have the decl marked DECL_NONALIASED? Alias analysis
> should not consider any address aliasing this (well, any with a mem_expr I
> guess).
No. RTL DSE gives up completely in all MEM_VOLATILE_P reads.
if ((MEM_ALIAS_SET (mem) == ALIAS_SET_MEMORY_BARRIER)
|| (MEM_VOLATILE_P (mem)))
{
if (dump_file && (dump_flags & TDF_DETAILS))
fprintf (dump_file, " adding wild read, volatile or barrier.\n");
add_wild_read (bb_info);
insn_info->cannot_delete = true;
return;
}
so it doesn't make into the alias oracle in any way, no idea why this has
been added in that form, seems to be a big hammer to me, but it is like that
(we obviously shouldn't try to replace_read those, but otherwise, I'd say
that whether a volatile or non-volatile read kills some store or not doesn't
really depend on whether it is volatile or not, but on the address;
I guess stage4 isn't the right time to change that though, it is this way
since r123530 when dse.c has been added).
Furthermore, the MEM_EXPR isn't always a DECL on which DECL_NONALIASED could be
applied, e.g. on x86_64-linux it is a MEM_REF built for the TLS memory slot.
Those were killing all the stores too.
Jakub
