[PATCH] D12706: Handle non-constant shifts in computeKnownBits, and use computeKnownBits for constant folding in InstCombine/Simplify

[hfinkel@anl.gov via llvm-commits]

hfinkel closed this revision.
hfinkel added a comment.

I apologize for the delay, and thanks for the reviews! r251146.


================
Comment at: lib/Analysis/ValueTracking.cpp:990
@@ +989,3 @@
+
+  computeKnownBits(I->getOperand(1), KnownZero, KnownOne, DL, Depth + 1, Q);
+
----------------
reames wrote:
> It would be more clearly correct to use the two temporaries for this calculation.  The current code is correct, but slightly confusing.  
I added an additional comment about this before I committed.

================
Comment at: lib/Analysis/ValueTracking.cpp:1015-1021
@@ +1014,9 @@
+
+  // If there are no compatible shift amounts, then we've proven that the shift
+  // amount must be >= the BitWidth, and the result is undefined. We could
+  // return anything we'd like, but we need to make sure the sets of known bits
+  // stay disjoint (it should be better for some other code to actually
+  // propagate the undef than to pick a value here using known bits).
+  if ((KnownZero & KnownOne) != 0)
+    KnownZero.clearAllBits(), KnownOne.clearAllBits();
+}
----------------
majnemer wrote:
> InstSimplify already has logic to handle shift amounts `>= bitwidth`.  Should we care whether or not `computeKnownBits` gives the same result?
I don't think that it can give the same result, because I can't return 'undef' here. We might be able to do that by returning conflicting KnownZero/KnownOne masks, but we'd need to fix some downstream code first.


http://reviews.llvm.org/D12706