[PATCH] D12745: [IndVars] Widen more comparisons for non-negative induction vars.
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 10 10:17:05 PDT 2015
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]]
Comment at: test/Transforms/IndVarSimplify/widen-loop-comp.ll:213
@@ +212,3 @@
+ %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?
More information about the llvm-commits