[PATCH] D18905: [ValueTracking] Improve isImpliedCondition for conditions with matching operands.

Chad Rosier via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 15 12:05:37 PDT 2016


mcrosier added a comment.

In http://reviews.llvm.org/D18905#402690, @bmakam wrote:

> Perhaps instcombine is a better place to catch this? I verified PR23333 could be catched by this approach. While improving http://reviews.llvm.org/D18841 I could handle PR23333 and almost all the cases mentioned in here except for test_gt, test_ltand test_eq.  I can easily handle these cases but I have currently limited http://reviews.llvm.org/D18841 to the immediate dominator of depth 1 in favor of walking the entire dominator tree for compile-time tradeoff. I am not sure which approach is better. Perhaps you could improve this to also handle PR23333 and other tests in http://reviews.llvm.org/D18841 or perhaps both solutions could co-exist.


I think the right plan of attack is to implement this logic here and then have other passes use the isImpliedCondition() API as is already done in SimplifyCFG, Jump Threading, and InstSimplify.

The logic added in this patch can prove that the first condition in PR23333 implies the second.  It will also catch the test cases in http://reviews.llvm.org/D18841 if isImpliedCondition() were used in InstCombine as you suggest.


http://reviews.llvm.org/D18905





More information about the llvm-commits mailing list