[PATCH] D94306: [LoopRotate] Use llvm.experimental.noalias.scope.decl for duplicating noalias metadata as needed

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

This is tricker than the unroll case, but also looks sound to me.



================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:479
+      // (0) : ... { D U }   // no relevant rotation for this part
+      // (1) : ... D' { U D }
+      // (2) : ... D' U' { D U }
----------------
// if D is part of OrigHeader


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:480
+      // (1) : ... D' { U D }
+      // (2) : ... D' U' { D U }
+      //
----------------
// if D and U are part of OrigHeader


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:496
+      for (Instruction *NAD : NoAliasDeclInstructions) {
+        LLVM_DEBUG(llvm::dbgs()
+                   << "  Cloning llvm.experimental.noalias.scope.decl:" << *NAD
----------------
llvm:: probably not needed.


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:503
+        // And move the original NAD to the NewHeader
+        NAD->moveBefore(NewHeaderInsertionPoint);
+      }
----------------
Is there any reason to insert a new one before NAD and then move NAD, rather then just inserting the new one at NewHeaderInsertionPoint?


================
Comment at: llvm/test/Transforms/LoopRotate/noalias.ll:44
+; CHECK:   store i32 42, i32* %_pA, align 16, !alias.scope !7
+; CHECK: for.end:
+
----------------
In line with the usual convention, this should be at the start of the `@test_02` function.


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

https://reviews.llvm.org/D94306