[PATCH] D18867: [IndVarSimplify] Eliminate zext of a signed IV when the IV is known to be non-negative

[Mitch Bodart via llvm-commits]

mbodart added inline comments.

================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:1309
@@ -1308,1 +1308,3 @@
+  if ((IsSigned ? isa<SExtInst>(DU.NarrowUse) : isa<ZExtInst>(DU.NarrowUse)) ||
+      (isa<ZExtInst>(DU.NarrowUse) && DU.NeverNegative)) {
     Value *NewDef = DU.WideDef;
----------------
The new check for isa<ZExtInst> seems unnecessary.
If DU.NeverNegative is true, it means DU.WideDef will have zeroes
in its upper bits regardless of whether it was created with a Zext or Sext.

This is the first time I've looked at this code, so I don't know if you can
simply delete this part of the check, or if you must at least verify
that DU.NarrowUse is some flavor of Extend (as opposed to an
arbitrary instruction).

================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:1402
@@ -1400,1 +1401,3 @@
+                           SE->getConstant(NarrowSCEV->getType(), 0)) ||
+      isKnownNonNegative(NarrowDef, NarrowDef->getModule()->getDataLayout());
   for (User *U : NarrowDef->users()) {
----------------
While the change seems correct, it raises the question of why isKnownPredicate is not
catching this case.  Have you looked into that?


http://reviews.llvm.org/D18867