[PATCH] D114917: [LoopInterchange] Enable loop interchange with multiple inner loop indvars

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopInterchange.cpp:698
     std::function<bool(Value *)> IsPathToIndVar;
-    IsPathToIndVar = [&InnerInduction, &IsPathToIndVar](Value *V) -> bool {
-      if (V == InnerInduction)
+    IsPathToIndVar = [this, &IsPathToIndVar](const Value *V) -> bool {
+      if (llvm::is_contained(InnerLoopInductions, V))
----------------
suggestion: rename to IsPathToInnerIndVar


================
Comment at: llvm/test/Transforms/LoopInterchange/interchangeable-innerloop-multiple-indvars.ll:21
+; CHECK-LABEL: @test1(
+; CHECK:    for.body:
+; CHECK:    [[INDVARS_OUTER:%.*]] = phi i64 [ [[INDVARS_OUTER_NEXT:%.*]], [[FOR_INC7:%.*]] ], [ 0, [[FOR_BODY_PREHEADER:%.*]] ]
----------------
please add check lines for all the BBs to show the control flow....and please use CHECK-NEXT.


================
Comment at: llvm/test/Transforms/LoopInterchange/interchangeable-innerloop-multiple-indvars.ll:96
+define void @test2() {
+; CHECK-LABEL: @test2(
+; CHECK:    for.body:
----------------
same comment as above


================
Comment at: llvm/test/Transforms/LoopInterchange/interchangeable-innerloop-multiple-indvars.ll:178
+; CHECK-LABEL: @test3(
+; CHECK:    for.body:
+; CHECK:    [[INDVARS_OUTER:%.*]] = phi i64 [ [[INDVARS_OUTER_NEXT:%.*]], [[FOR_INC7:%.*]] ], [ 0, [[FOR_BODY_PREHEADER:%.*]] ]
----------------
same comment as above


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114917/new/

https://reviews.llvm.org/D114917