[PATCH] D21449: Target independent codesize heuristics for Loop Idiom Recognition

[Chad Rosier via llvm-commits]

mcrosier added inline comments.

================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:137
@@ -123,3 +136,3 @@
                                const SCEVAddRecExpr *Ev, const SCEV *BECount,
-                               bool NegStride);
+                               bool NegStride, bool IsLoopMemset=false);
   bool processLoopStoreOfLoopLoad(StoreInst *SI, const SCEV *BECount);
----------------
Please add a space on both sides of the '='.

================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:466
@@ -448,2 +465,3 @@
         continue;
+
       MadeChange = true;
----------------
Please don't add whitespace.

================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:763
@@ -744,1 +762,3 @@
 
+  // When compiling for codesize we avoid memset idiom recognition for a
+  // multi-block loop unless it is either a loop_memset idiom or
----------------
This code is duplicated verbatim below.  I suggest adding a static helper function to avoid such duplication.

================
Comment at: test/Transforms/LoopIdiom/lir-heurs-multi-block-loop.ll:14
@@ +13,3 @@
+; CHECK-LABEL: LoopMemset
+; CHECK: memset
+;
----------------
I don't understand what this (or the next test) is testing.  I believe you need two memsets that can be merged together in both cases to test this properly.


https://reviews.llvm.org/D21449