[PATCH] D30352: [JumpThread] Use AA in SimplifyPartiallyRedundantLoad()

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

In https://reviews.llvm.org/D30352#688924, @rengolin wrote:

> In https://reviews.llvm.org/D30352#688862, @dberlin wrote:
>
> > In the future, can you please analyze why this is happening?
> >  This is another example of a test case that is unrelated to the pattern you are trying to optimize.  That means if this broke optimizing this tomorrow, we'd have to run spec again and analyze it.  
> >  We also really need to understand what optimizations need to be happening, not just randomly implement stuff in various passes because it increases SPEC
>
>
> Hi Daniel,
>
> I didn't see this as a "new optimisation that needs doing", just as an "omitted analysis that got finally added", and obviously, if you have AA, then alias analysis will be done and the case where that matters will be better.
>
> IIUC, the test is just to make sure it doesn't get removed or changed in the future.
>
> The comment about SPEC improvement, from my POV was irrelevant, given that this is an obvious win.
>
> Unless I'm missing something obvious here, of course... :)
>
> --renato


Hey Renato, 
This is not the first patch in this line of extending the PRE in jump threading so I get that you may be missing context. :)
So far we've extended and then made this optimization more expensive without a single real test we care about, and every single testcase added is already caught by other passes.  Rather than simply make every pass do everything, because it seems to improve spec score, I would really like to see us actually analyze and understand why it is improving spec and whether extending and making this optimization is really the right plan. I agree that otherwise this particular change is innocuous,  but I asked for analysis and real cases for the last patches and haven't gotten them yet.

Obviously, if analysis shows this pass is the right thing to improve, we should do it. But I would like to see that happen before we keep going.


https://reviews.llvm.org/D30352