[PATCH] D34566: [loop idiom Recognition] support memcpy for multiple consecutive loads and stores

[Mandeep Singh Grang via Phabricator via llvm-commits]

mgrang added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:161
                                PHINode *CntPhi, Value *Var);
+
   bool recognizeAndInsertCTLZ();
----------------
Extra newline.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:371
 
+
   // Don't convert stores of non-integral pointer types to memsets (which stores
----------------
Extra newline.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:458
                            : LegalStoreKind::Memcpy;
+
   }
----------------
Ditto.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:497
       break;
+
     }
----------------
Ditto.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:919
+  // all of the pairs of stores that follow each other.
+  for (unsigned i = 0, e = SL.size(); i < e; ++i) {
+    assert(SL[i]->isUnordered() && "Expected only non-volatile non-ordered stores.");
----------------
Use i != e instead of i < e.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:951
+
+    for(k=i+1;;k=k+delta){
+      if (k == e) {
----------------
This loop looks a bit hacky. Can this be re-written in a better way?


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:954
+        delta = -1;
+	if(i==0)
+          k=0;
----------------
Check indentation.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1204
   return false;
 }
 bool LoopIdiomRecognize::runOnNoncountableLoop() {
----------------
Newline removed.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1607
   Value *Ops[] = {Val, ZeroCheck ? IRBuilder.getTrue() : IRBuilder.getFalse()};
   Type *Tys[] = {Val->getType()};
   Module *M = IRBuilder.GetInsertBlock()->getParent()->getParent();
----------------
Newline removed.


================
Comment at: test/Transforms/LoopIdiom/memcpy_structPattern.ll:12
+
+ at bar3.g = private unnamed_addr constant [14 x %struct.foo] [%struct.foo { i32 2, i32 3 }, %struct.foo { i32 3, i32 4 }, %struct.foo { i32 1, i32 8 }, %struct.foo { i32 3, i32 2 }, %struct.foo { i32 4, i32 5 }, %struct.foo { i32 4, i32 5 }, %struct.foo { i32 1, i32 2 }, %struct.foo { i32 2, i32 3 }, %struct.foo { i32 3, i32 4 }, %struct.foo { i32 1, i32 8 }, %struct.foo { i32 3, i32 2 }, %struct.foo { i32 4, i32 5 }, %struct.foo { i32 4, i32 5 }, %struct.foo { i32 1, i32 2 }], align 16
+ at bar4.g = private unnamed_addr constant [14 x %struct.foo1] [%struct.foo1 { i32 2 }, %struct.foo1 { i32 4 }, %struct.foo1 { i32 5 }, %struct.foo1 { i32 6 }, %struct.foo1 { i32 9 }, %struct.foo1 { i32 5 }, %struct.foo1 { i32 1 }, %struct.foo1 { i32 2 }, %struct.foo1 { i32 3 }, %struct.foo1 { i32 3 }, %struct.foo1 { i32 5 }, %struct.foo1 { i32 7 }, %struct.foo1 { i32 8 }, %struct.foo1 { i32 19 }], align 16
----------------
Can the test case be simplified/minimized by removing unwanted/extra attributes?


https://reviews.llvm.org/D34566