[PATCH] Improve RTL DSE with -fstack-protector* (PR rtl-optimization/88796)

[Richard Biener]

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.