On Fri, 4 Nov 2016, Jakub Jelinek wrote:
> On Fri, Nov 04, 2016 at 01:47:36PM +0100, Richard Biener wrote:
> > I'm not sure the current code handled this correctly? At least I
> > see nothing on the GIMPLE level that would disallow the DSE:
> >
> > int foo ()
> > {
> > char a[256];
> > a[0] = 'a';
> > a[1] = 'b';
> > int __attribute__((const)) (*fn) (void) = (int (*) (void)) &a[0];
> > return fn ();
> > }
> >
> > even with pure or regular fn this gets DSEd, the call is appearantly
> > not an escape point for &a.
> >
> > That is, for DSE it would only matter for locals and/or const
> > function calls (if stores after the call make the earlier appear dead).
> >
> > We did have bugs like PR70128 fixed though (but that was "global memory").
> > It might be still broken if the patched function is const and we
> > unpatch right after calling...
> >
> > So to answer, yes, I think we want to treat this conservatively
> > (but we may have existing bugs here).
>
> So like the variant patch I've just posted?
That doesn't handle
int __attribute__((const,noinline))
foo ()
{
return 1;
}
int bar()
{
*((int *)foo) + 4 = 2;
int ret = foo ();
*((int *)foo) + 4 = 1;
return ret;
}
right? patching foo to return 2, calling foo and then unpatching it?
> > > In any case, the second hunk fixes an important DSE bug that just got
> > > revealed by the former change.
> >
> > Do you have a testcase for the (wrong-code?) bug?
>
> FAIL: gcc.dg/torture/pr67690.c -Os execution test
>
> that appeared on i686-linux with the first hunk without the second hunk.
Ah ok.
Richard.
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
