[PATCH] D13717: [IndVars] Have `cloneArithmeticIVUser` guess better

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 15 15:34:28 PDT 2015


reames added a comment.

I have only minor comments, but I'm not familiar enough with the surrounding code to spot any subtle mistakes.  I'll say LGTM, but take that for what it's worth.


================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:1061
@@ +1060,3 @@
+  if (!GuessNonIVOperand(SignExtend)) {
+    if (!GuessNonIVOperand(!SignExtend))
+      return nullptr;
----------------
I'd restructure this to put the assignment before the second guess.  It would make it more obvious what's going on.

================
Comment at: lib/Transforms/Scalar/IndVarSimplify.cpp:1068
@@ +1067,3 @@
+                   ? WideDef
+                   : getExtend(NarrowUse->getOperand(0), WideType, SignExtend,
+                               NarrowUse);
----------------
The fact this function is also called getExtend is confusing.  getExtend != GetExtend?  


http://reviews.llvm.org/D13717





More information about the llvm-commits mailing list