[PATCH] D10477: Expand loop reroll to handle loop with multiple induction variables and negative increment -part 1/3

hfinkel at anl.gov hfinkel at anl.gov
Fri Jul 24 05:19:12 PDT 2015


hfinkel added inline comments.

================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:1278
@@ -1279,3 +1277,3 @@
       (SE->getAddRecExpr(Start,
-                         SE->getConstant(RealIVSCEV->getType(), 1),
+                         SE->getConstant(RealIVSCEV->getType(), Negative ? -1 : 1),
                          L, SCEV::FlagAnyWrap));
----------------
Line is too long.

================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:1296
@@ -1297,3 +1295,3 @@
           const SCEV *ICMinus1SCEV =
-            SE->getMinusSCEV(ICSCEV, SE->getConstant(ICSCEV->getType(), 1));
+            SE->getMinusSCEV(ICSCEV, SE->getConstant(ICSCEV->getType(), Negative ? -1 : 1));
 
----------------
Line is too long.

================
Comment at: test/Transforms/LoopReroll/negative.ll:2
@@ +1,3 @@
+; RUN: opt -S  -loop-reroll   %s | FileCheck %s
+; REQUIRES: asserts
+target triple = "aarch64--linux-gnu"
----------------
Remove this. There is no valid reason why a test like this needs to be asserts-mode only. And it is really important that we have as much testing as possible for the NDEBUG builds (even if that were to require using more regexes in the test). That having been said, what James said was not quite right. The NDEBUG opt does not remove all variable names (certain passes did that, or used to, but that's another matter). What is true is that NDEBUG Clang does not produce variable names for locals.



Repository:
  rL LLVM

http://reviews.llvm.org/D10477







More information about the llvm-commits mailing list