[PATCH] D29781: Add alias canonicalization pass when preparing for ThinLTO

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: lib/Transforms/Utils/CanonicalizeAliases.cpp:35
+  if (BitCastOperator *BC = dyn_cast<BitCastOperator>(V)) {
+    for (Use &U : BC->materialized_uses()) {
+      return userIsAlias(U.getUser());
----------------
Why `materialized_uses`?


================
Comment at: lib/Transforms/Utils/CanonicalizeAliases.cpp:36
+    for (Use &U : BC->materialized_uses()) {
+      return userIsAlias(U.getUser());
+    }
----------------
That deserves a comment. I'd even desugarize the loop with an assert that there is only one use as "documentation" of the invariant you're relying on here. (alternatively, or at the same time, add a comment)


================
Comment at: lib/Transforms/Utils/CanonicalizeAliases.cpp:99
+    // RAUW all uses of original aliasee with the new alias.
+    replaceAllUsesWithExceptAliases(GO, NewGA);
+
----------------
Instead of reimplementing a custom RAUW (the duplication isn't great here), would it be possible to perform the regular one, and then reattach the aliases?



================
Comment at: lib/Transforms/Utils/CanonicalizeAliases.cpp:110
+// Legacy pass that canonicalizes aliases.
+class CanonicalizeAliasesLegacyPass : public ModulePass {
+
----------------
New PM version of the pass?


https://reviews.llvm.org/D29781