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

Arthur Eubanks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 5 16:31:19 PST 2022


aeubanks added a comment.

it'd be good to put this through llvm-compile-time-tracker. a branch with a commit that turns on `-ftrivial-auto-var-init`, then this commit, to see how much this adds to compile time when using `-ftrivial-auto-var-init`

it'd be interesting to remove the "auto-init" restriction and see what effects on compile time this has in general, and maybe benchmark results



================
Comment at: llvm/lib/Transforms/Utils/MoveAutoInit.cpp:24-25
+#include "llvm/IR/IntrinsicInst.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Pass.h"
+#include "llvm/Support/CommandLine.h"
----------------
can remove these now that there's no legacy pass


================
Comment at: llvm/lib/Transforms/Utils/MoveAutoInit.cpp:103-104
+
+    // Traverse the cfg to detect cycles involving UsersDominator. We don't want
+    // a direct path from the entry point to UsersDominator, without back edges.
+    SmallPtrSet<BasicBlock *, 8> TransitiveSuccessors;
----------------
this sentence doesn't make sense to me


================
Comment at: llvm/lib/Transforms/Utils/MoveAutoInit.cpp:112
+      if (CurrBB == UsersDominator)
+        // No early exit because we ant to compute the full set of transitive
+        // successors.
----------------
want


================
Comment at: llvm/test/Transforms/MoveAutoInit/branch.ll:9
+
+define dso_local void @foo(i32 noundef %x) {
+; CHECK-LABEL: @foo(
----------------
test nit: could you remove `dso_local`/`noundef`


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

https://reviews.llvm.org/D137707



More information about the llvm-commits mailing list