[PATCH] D30004: [InstCombin] Continue scanning loaded values through single predecessors

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 15 13:24:28 PST 2017


dberlin added a comment.

In https://reviews.llvm.org/D30004#677832, @junbuml wrote:

> I also observed this change was applied pretty widely in my spec2000/2006 and had impacts on the final output code, which means the opportunities are exposed in InstCombin and they are not handled in other places in the whole pipeline;


We definitely should investigate that.
Because that is very worrying, and we should definitely fix those deficiencies. But without seeing them,i'm not sure i'd fix them in instcombine, and the general preference would be "make pass that should be catching it, catch it

>   let me try to reduce such case from my spec tests. I added the testcase to describe what this patch do in instcombine itself, not to show the case which can only be handled in instcombine. 

Sure, but i'd like us to try to test stuff that is specific to the pass.
Otherwise, we'd even be better off unifying these tests in a single place and testing every pass that can do it.
(IE we have no reason to copy the same test 20 times in the codebase)
;)


https://reviews.llvm.org/D30004





More information about the llvm-commits mailing list