On Fri, 15 Nov 2013, Jakub Jelinek wrote:
> On Fri, Nov 15, 2013 at 02:56:51PM +0100, Richard Biener wrote:
> > Now that there is (finally :() a wrong-code testcase for the
> > PR54570 issue we can no longer ignore it (bah). So the following
> > tries to paper over the fact that object-size sucks and disables
> > value-numbering of equal addresses the same before that pass
> > had a chance to finally look at the structure of the addresses.
> >
> > To make this "fix" suck less I moved the object-size pass before
> > the final FRE pass runs which is after IPA inlining and the
> > propagation of constants and addresses. You won't catch
> > any "improvements" you'd get by memory CSE opportunities that
> > IPA inlining exposes, but you cannot have everything here.
>
> If it doesn't regress anything in the testsuite, I guess that is ok.
>
> > (IMHO object-size would should run during early optimizations)
>
> It can't, because inlining and some limited cleanup afterwards is essential
> for it. Otherwise you'd regress not just for __builtin_object_size (x, 1),
> which admittedly is problematic since the introduction of MEM_REFs and
> various other changes, but also for __builtin_object_size (x, 0), which
> would be much more important.
>
> As discussed earlier, perhaps instead of checking cfun->after_inlining
> you could just introduce a new flag whether cfun contains any
> __builtin_object_size (x, {1,3}) calls, initialized by the gimplifier,
> propagated by the inliner and finally cleared again by objsz pass.
But you'd need to pessimistically initialize it because if you inline
into a function with __builtin_object_size you may not previously
optimize. You can of course analyze the cgraph to clear it for
functions that cannot end up being inlined into such function.
So that's effectively the same as ->after_inlining
minus losing the optimization that didn't end up with
__builtin_object_size after that.
Not sure if it's worth all the trouble. Arriving at a better
design for computing __builtin_object_size would be better ;)
(not that I have one)
> Of course, if moving objsz earlier seems to work, it could stay where you
> put it, but the flag could make it clearer why you want to avoid certain
> optimizations.
Well, all object-size testcases are pretty simplistic right now and
don't trigger the IPA inliner for example.
> > Bootstrap / regtest pending on x86_64-unknown-linux-gnu.
> >
> > Similar candidate for the 4.8 branch.
>
> Please wait sufficiently long for trunk issues before you backport.
Of course.
Richard.
