On Fri, 11 Jan 2019, Jakub Jelinek wrote: > 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.
Ah, OK. Well, the patch is OK then I suppose. Thanks, Richard.