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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.
Herald added a subscriber: dexonsmith.

For canonicalization this seems like a better approach. It walks the alias operands so you don't hit the issue with alias vs non-alias users. I wrote it as a function inside LTO but it could easily be moved into a pass.

  diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
  index 926c419e34a..bbca8f5b6e7 100644
  --- a/llvm/lib/LTO/LTOBackend.cpp
  +++ b/llvm/lib/LTO/LTOBackend.cpp
  @@ -439,8 +439,49 @@ Error lto::backend(Config &C, AddStreamFn AddStream,
     return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
   }
   
  -static void dropDeadSymbols(Module &Mod, const GVSummaryMapTy &DefinedGlobals,
  +static Constant *resolveAliases(Constant *C) {
  +  if (auto *GA = dyn_cast<GlobalAlias>(C))
  +    return resolveAliases(GA->getAliasee());
  +
  +  if (auto *GO = dyn_cast<GlobalObject>(C)) {
  +    if (!GO->hasLocalLinkage()) {
  +      // Replace GO with an alias that points to it.
  +      auto *GA = GlobalAlias::create("", GO);
  +      GA->setLinkage(GO->getLinkage());
  +      GA->copyAttributesFrom(GO);
  +      GA->takeName(GO);
  +      GO->replaceAllUsesWith(GA);
  +      GA->setAliasee(GO);
  +      GO->setLinkage(GlobalValue::PrivateLinkage);
  +
  +      // This will end up giving a suffixed name to GO.
  +      GO->setName(GA->getName());
  +    }
  +    return GO;
  +  }
  +
  +  auto *CE = dyn_cast<ConstantExpr>(C);
  +  if (!CE)
  +    return C;
  +
  +  std::vector<Constant *> Ops;
  +  for (Use &U : CE->operands())
  +    Ops.push_back(resolveAliases(cast<Constant>(U)));
  +  return CE->getWithOperands(Ops);
  +}
  +
  +static void dropDeadSymbols(Module &Mod, const Triple &TT,
  +                            const GVSummaryMapTy &DefinedGlobals,
                               const ModuleSummaryIndex &Index) {
  +  for (GlobalAlias &GA : Mod.aliases()) {
  +    // Don't handle COFF weak externals, since unlike regular aliases they
  +    // operate on the symbol level.
  +    if (GA.hasWeakLinkage() && TT.isOSBinFormatCOFF())
  +      continue;
  +
  +    GA.setAliasee(resolveAliases(GA.getAliasee()));
  +  }
  +
     std::vector<GlobalValue*> DeadGVs;
     for (auto &GV : Mod.global_values())
       if (GlobalValueSummary *GVS = DefinedGlobals.lookup(GV.getGUID()))
  @@ -488,7 +529,7 @@ Error lto::thinBackend(Config &Conf, unsigned Task, AddStreamFn AddStream,
   
     renameModuleForThinLTO(Mod, CombinedIndex);
   
  -  dropDeadSymbols(Mod, DefinedGlobals, CombinedIndex);
  +  dropDeadSymbols(Mod, TM->getTargetTriple(), DefinedGlobals, CombinedIndex);
   
     thinLTOResolvePrevailingInModule(Mod, DefinedGlobals);
   



================
Comment at: lib/Transforms/Utils/CanonicalizeAliases.cpp:58
+  for (Use &U : Old->materialized_uses()) {
+    if (userIsAlias(U.getUser()))
+      continue;
----------------
I don't think this is entirely correct. A bitcast can have both alias and non-alias uses, so you would need to create different ConstantExprs for each. It actually seems simpler to visit the alias's operands than the use list.


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

https://reviews.llvm.org/D29781