[PATCH] D29781: Add alias canonicalization pass when preparing for ThinLTO
Mehdi AMINI via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 13 18:22:06 PST 2017
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
More information about the llvm-commits
mailing list