[PATCH] D12745: [IndVars] Widen more comparisons for non-negative induction vars.

[Philip Reames via llvm-commits]

reames added a subscriber: reames.
reames added a comment.

Minor comments.  The direction of the change looks right to me.  I'm not comfortable giving a LGTM on this given I don't know the area well enough.


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:849
@@ -848,1 +848,3 @@
 
+  // True if the narrow induction variable is never negative.
+  bool NeverNegative;
----------------
A brief bit about what we can use this fact for would be good in the comment.  

================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:1265
@@ -1246,1 +1264,3 @@
 
+  NeverNegative = SE->isKnownPredicate(ICmpInst::ICMP_SGE, AddRec,
+                                       SE->getConstant(AddRec->getType(), 0));
----------------
Why not just SE->isKnownNonNegative(AddRec)?

================
Comment at: test/Transforms/IndVarSimplify/widen-loop-comp.ll:200
@@ +199,3 @@
+; CHECK: [[B_SEXT:%[a-z0-9]+]] = sext i32 %b to i64
+; CHECK: for.cond:
+; CHECK: icmp sle i64 %indvars.iv, [[B_SEXT]]
----------------
CHECK_LABEL

================
Comment at: test/Transforms/IndVarSimplify/widen-loop-comp.ll:213
@@ +212,3 @@
+for.body:
+  %idxprom = zext i32 %i.0 to i64
+  %arrayidx = getelementptr inbounds i32, i32* %a, i64 %idxprom
----------------
It isn't obvious to me whether we chose a sext or a sext for the induction variable in this loop.  We could do either base on order of visitation.  Could you clarify this?

Also, can you add tests for the full 2 x 2 matrix of signed/unsigned widdening x signed/unsigned compare?


http://reviews.llvm.org/D12745