Add a test case to show isKnownNonZero() returns correctly; NFC

[junbuml]

Summary

Added a test case just to make sure that isKnownNonZero() returns false
when we cannot guarantee that a ConstantExpr is a non-zero constant.

[mssimpso]

Jun,

This makes sense to me. But I'm curious if you've seen any changes with this because it doesn't look to be functionally identical, due to the extra floating point checking done in isZeroValue. While you're here, it would be nice to fix the comment for the function to mention the constants.

[junbuml]

This makes sense to me. But I'm curious if you've seen any changes with this because it doesn't look to be functionally identical, due to the extra floating point checking done in isZeroValue. While you're here, it would be nice to fix the comment for the function to mention the constants.

I am sending this out to get any early feedback. Since this is not a NFC, I need to check performance. This is also missing test cases that I will add soon.

[nlewycky]

What about a ConstantExpr like "and(ptrtoint(@global), 32)"? We don't know the 5th bit of the address of @global, so C->isZeroValue() returns false (since we don't know it's zero), but isKnownNonZero must also return false (since we don't know it's non-zero). With your patch, we would incorrectly return true.

You are replacing "is known to be non-zero" with "is not known to be zero" which are not quite the same thing for all Constants. Please send out a patch which adds a testcase that would fail if someone were to propose this change in the future.

[junbuml]

Based on Nick's comment, I just added a test case to prevent any similar changes in future.
Thanks Nick for the comment.

[mcrosier]

LGTM, with minor nits.

test/Transforms/InstSimplify/compare.ll

336 ↗

(On Diff #47648)

Please drop "expect to".

Also, please maximize 80-column.

[junbuml]

Addressed Chad's comment.

lib/Analysis/ValueTracking.cpp

static bool rangeMetadataExcludesValue(MDNode* Ranges,
   return true;
 }
 
 /// Return true if the given value is known to be non-zero when defined.
 /// For vectors return true if every element is known to be non-zero when
 /// defined. Supports values with integer or pointer type and vectors of
 /// integers.
 bool isKnownNonZero(Value *V, unsigned Depth, const Query &Q) {
-  if (Constant *C = dyn_cast<Constant>(V)) {
-    if (C->isNullValue())
-      return false;
-    if (isa<ConstantInt>(C))
-      // Must be non-zero due to null test above.
-      return true;
-    // TODO: Handle vectors
-    return false;
-  }
+  if (Constant *C = dyn_cast<Constant>(V))
+    return !C->isZeroValue();
 
   if (Instruction* I = dyn_cast<Instruction>(V)) {
     if (MDNode *Ranges = I->getMetadata(LLVMContext::MD_range)) {
       // If the possible ranges don't contain zero, then the value is
       // definitely non-zero.
       if (IntegerType* Ty = dyn_cast<IntegerType>(V->getType())) {
         const APInt ZeroValue(Ty->getBitWidth(), 0);
         if (rangeMetadataExcludesValue(Ranges, ZeroValue))