[PATCH] D33567: [InstCombine] Pass the DominatorTree and AssumptionCache to a few calls to isKnownPositive, isKnownNegative, and isKnownNonZero

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 26 10:48:55 PDT 2017


hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D33567#765886, @craig.topper wrote:

> I was operating under the assumption that this was an accident. Most other places use the InstCombine specific wrappers around computeKnownBits so always pass the the dominatortree and assumption cache. If there's a reason for these places to be different then that should be commented.


I agree. Consistency is king in this regard, unless we have a specific (commented) reason to deviate. Otherwise, it makes it very difficult to reason about what the calls will do. When the original changes were made, all of these calls were swept in, and specific justification should not be required to correct oversights in new/other code.


https://reviews.llvm.org/D33567





More information about the llvm-commits mailing list