[PATCH] D137707: Move "auto-init" instructions to the dominator of their users

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/MoveAutoInit.cpp:60
+
+  MemoryUseOrDef *IMA = MSSA.getMemoryAccess(I);
+  auto &AA = MSSA.getAA();
----------------
To assert non-null, just use a reference.


================
Comment at: llvm/lib/Transforms/Utils/MoveAutoInit.cpp:72
+    MemoryAccess *MA = WorkList.pop_back_val();
+    if (!Visited.insert(MA).second)
+      continue;
----------------
For a work list, we should add elements to Visited before pushing elements to WorkList, otherwise WorkList may contain duplicates.


================
Comment at: llvm/lib/Transforms/Utils/MoveAutoInit.cpp:103
+  for (Instruction &I : EntryBB) {
+
+    // FIXME: this guards against generalization of this pass to any statement.
----------------
unneeded blank line


================
Comment at: llvm/lib/Transforms/Utils/MoveAutoInit.cpp:137
+    // but we can insert it in the non back-edge predecessors, if it exists.
+
+    if (HasCycle) {
----------------
unneeded blank line


================
Comment at: llvm/lib/Transforms/Utils/MoveAutoInit.cpp:149
+      for (BasicBlock *Pred : predecessors(UsersDominatorHead)) {
+
+        // If one of the predecessor of the dominator also transitively is a
----------------
unneeded blank line


================
Comment at: llvm/test/Transforms/MoveAutoInit/clobber.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -S -passes='move-auto-init' | FileCheck %s
----------------
Add a file-level comment what this test is about.


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

https://reviews.llvm.org/D137707