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

Renato Golin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 1 02:41:06 PST 2017


rengolin added a comment.

In https://reviews.llvm.org/D30352#688933, @dberlin wrote:

> 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.


Right, it seems I have jumped the gun on what I thought it was just an obvious patch. Apologies.

> 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.

I totally agree. We're already slower than GCC for usually less performance. I agree having a better analysis of at least execution time worth doing.

Though, I wonder how much this one patch would fare (and be wrongly picked upon) across all previous patches. Not that this should stop any further investigation before commit, but that we maybe should look at the bigger picture (if this has been happening consistently) and plot a graph with relative performance vs. compile time for a particular set of patches...

Makes sense?

--renato


https://reviews.llvm.org/D30352





More information about the llvm-commits mailing list