Utility functions for appending to llvm.used/llvm.compiler.used

[pcc]

lib/LTO/LTOCodeGenerator.cpp

368

This could be a std::vector.

lib/LTO/UpdateCompilerUsed.cpp

55

This could also be a std::vector, then there'd be no need to rebuild it.

[mehdi_amini]

lib/LTO/UpdateCompilerUsed.cpp

55

How do you prevent duplicate?

[pcc]

lib/LTO/UpdateCompilerUsed.cpp

55

findLibCallsAndAsm is the only function that adds globals to LLVMUsed. The only call sites are in findInModule which cannot pass duplicates. We can easily change findLibCallsAndAsm to add the global at most once.

[mehdi_amini]

lib/LTO/UpdateCompilerUsed.cpp

55

But this is "updating" the list, so even if findLibCallsAndAsm adds the globals at most once, they can already be in the list, what did I miss?

[eugenis]

lib/LTO/UpdateCompilerUsed.cpp

55

The only user start with an empty list.

[mehdi_amini]

lib/LTO/UpdateCompilerUsed.cpp

55

OK I see, because of the way you split it, but see below...

lib/Transforms/Utils/ModuleUtils.cpp

104

What if some values that you add here are already in the global variable initializer?

[eugenis]

lib/Transforms/Utils/ModuleUtils.cpp

104

We'll end up with duplicates. This is how the current code works.

Btw, is not SmallPtrSet unstable in the "large mode", when it is backed by hash table? Do we not care about the order of llvm.used entries being stable?

[pcc]

LGTM

[mehdi_amini]

lib/LTO/UpdateCompilerUsed.cpp

117

This is a set, which ensure no duplicate.

lib/Transforms/Utils/ModuleUtils.cpp

104

I don't believe it is how the current code work. I'll point it inline.

[eugenis]

lib/LTO/UpdateCompilerUsed.cpp

117

Now I may be missing something, but this set is converted to a vector before the current llvm.compiler.used is read. There is now deduplication between the current entries of llvm.compiler.used and the new elements. And, as Peter explained above, the new elements are unique among themselves.

[mehdi_amini]

lib/LTO/UpdateCompilerUsed.cpp

117

You're right. However it is a bug I introduced when refactoring this in http://reviews.llvm.org/D19000 ; the original code was turned the set into a vector only after appending the existing elements. So we should fix this bug and not simplify the code assuming the buggy behavior is expected.
I don't expect this API, updateCompilerUsed, to introduce a duplicate here.

[eugenis]

PTAL

[mehdi_amini]

Thanks.

[eugenis]

r285143
Thanks for the review.

include/llvm/Transforms/InstrProfiling.h

struct PerFunctionProfileData {
     uint32_t NumValueSites[IPVK_Last + 1];
     GlobalVariable *RegionCounters;
     GlobalVariable *DataVar;
     PerFunctionProfileData() : RegionCounters(nullptr), DataVar(nullptr) {
       memset(NumValueSites, 0, sizeof(uint32_t) * (IPVK_Last + 1));
     }
   };
   DenseMap<GlobalVariable *, PerFunctionProfileData> ProfileDataMap;
-  std::vector<Value *> UsedVars;
+  std::vector<GlobalValue *> UsedVars;
   std::vector<GlobalVariable *> ReferencedNames;
   GlobalVariable *NamesVar;
   size_t NamesSize;
 
   bool isMachO() const;
 
   /// Get the section name for the counter variables.
   StringRef getCountersSection() const;

include/llvm/Transforms/Utils/ModuleUtils.h

std::pair<Function *, Function *> createSanitizerCtorAndInitFunctions(
     Module &M, StringRef CtorName, StringRef InitName,
     ArrayRef<Type *> InitArgTypes, ArrayRef<Value *> InitArgs,
     StringRef VersionCheckName = StringRef());
 
 /// Rename all the anon globals in the module using a hash computed from
 /// the list of public globals in the module.
 bool nameUnamedGlobals(Module &M);
 
+/// \brief Adds global values to the llvm.used list.
+void appendToUsed(Module &M, ArrayRef<GlobalValue *> Values);
+
+/// \brief Adds global values to the llvm.compiler.used list.
+void appendToCompilerUsed(Module &M, ArrayRef<GlobalValue *> Values);
+
 } // End llvm namespace
 
 #endif //  LLVM_TRANSFORMS_UTILS_MODULEUTILS_H

lib/LTO/LTOCodeGenerator.cpp

 #include "llvm/Target/TargetLowering.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Target/TargetRegisterInfo.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/IPO/Internalize.h"
 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
 #include "llvm/Transforms/ObjCARC.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
 #include <system_error>
 using namespace llvm;
 
 const char* LTOCodeGenerator::getVersionString() {
 #ifdef LLVM_VERSION_INFO
   return PACKAGE_NAME " version " PACKAGE_VERSION ", " LLVM_VERSION_INFO;
 #else
   return PACKAGE_NAME " version " PACKAGE_VERSION;

 }
 
 // If a linkonce global is present in the MustPreserveSymbols, we need to make
 // sure we honor this. To force the compiler to not drop it, we add it to the
 // "llvm.compiler.used" global.
 void LTOCodeGenerator::preserveDiscardableGVs(
     Module &TheModule,
     llvm::function_ref<bool(const GlobalValue &)> mustPreserveGV) {
-  SetVector<Constant *> UsedValuesSet;
-  if (GlobalVariable *LLVMUsed =
-          TheModule.getGlobalVariable("llvm.compiler.used")) {
-    ConstantArray *Inits = cast<ConstantArray>(LLVMUsed->getInitializer());
-    for (auto &V : Inits->operands())
-      UsedValuesSet.insert(cast<Constant>(&V));
-    LLVMUsed->eraseFromParent();
-  }
-  llvm::Type *i8PTy = llvm::Type::getInt8PtrTy(TheModule.getContext());
+  std::vector<GlobalValue *> Used;
   auto mayPreserveGlobal = [&](GlobalValue &GV) {
     if (!GV.isDiscardableIfUnused() || GV.isDeclaration() ||
-        !mustPreserveGV(GV)) 
+        !mustPreserveGV(GV))
       return;
     if (GV.hasAvailableExternallyLinkage())
       return emitWarning(
           (Twine("Linker asked to preserve available_externally global: '") +
            GV.getName() + "'").str());
     if (GV.hasInternalLinkage())
       return emitWarning((Twine("Linker asked to preserve internal global: '") +
                    GV.getName() + "'").str());
-    UsedValuesSet.insert(ConstantExpr::getBitCast(&GV, i8PTy));
+    Used.push_back(&GV);
   };
   for (auto &GV : TheModule)
     mayPreserveGlobal(GV);
   for (auto &GV : TheModule.globals())
     mayPreserveGlobal(GV);
   for (auto &GV : TheModule.aliases())
     mayPreserveGlobal(GV);
 
-  if (UsedValuesSet.empty())
+  if (Used.empty())
     return;
 
-  llvm::ArrayType *ATy = llvm::ArrayType::get(i8PTy, UsedValuesSet.size());
-  auto *LLVMUsed = new llvm::GlobalVariable(
-      TheModule, ATy, false, llvm::GlobalValue::AppendingLinkage,
-      llvm::ConstantArray::get(ATy, UsedValuesSet.getArrayRef()),
-      "llvm.compiler.used");
-  LLVMUsed->setSection("llvm.metadata");
+  appendToCompilerUsed(TheModule, Used);
 }
 
 void LTOCodeGenerator::applyScopeRestrictions() {
   if (ScopeRestrictionsDone)
     return;
 
   // Declare a callback for the internalize pass that will ask for every
   // candidate GlobalValue if it can be internalized or not.

lib/LTO/UpdateCompilerUsed.cpp

 
 #include "llvm/LTO/legacy/UpdateCompilerUsed.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/Target/TargetLowering.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 #include "llvm/Transforms/IPO/Internalize.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
 
 using namespace llvm;
 
 namespace {
 
 // Helper class that collects AsmUsed and user supplied libcalls.
 class PreserveLibCallsAndAsmUsed {
 public:
   PreserveLibCallsAndAsmUsed(const StringSet<> &AsmUndefinedRefs,
                              const TargetMachine &TM,
-                             SmallPtrSetImpl<const GlobalValue *> &LLVMUsed)
+                             std::vector<GlobalValue *> &LLVMUsed)
       : AsmUndefinedRefs(AsmUndefinedRefs), TM(TM), LLVMUsed(LLVMUsed) {}
 
-  void findInModule(const Module &TheModule) {
+  void findInModule(Module &TheModule) {
     initializeLibCalls(TheModule);
-    for (const Function &F : TheModule)
+    for (Function &F : TheModule)
       findLibCallsAndAsm(F);
-    for (const GlobalVariable &GV : TheModule.globals())
+    for (GlobalVariable &GV : TheModule.globals())
       findLibCallsAndAsm(GV);
-    for (const GlobalAlias &GA : TheModule.aliases())
+    for (GlobalAlias &GA : TheModule.aliases())
       findLibCallsAndAsm(GA);
   }
 
 private:
   // Inputs
   const StringSet<> &AsmUndefinedRefs;
   const TargetMachine &TM;
 
   // Temps
   llvm::Mangler Mangler;
   StringSet<> Libcalls;
 
   // Output
-  SmallPtrSetImpl<const GlobalValue *> &LLVMUsed;
+  std::vector<GlobalValue *> &LLVMUsed;
 
   // Collect names of runtime library functions. User-defined functions with the
   // same names are added to llvm.compiler.used to prevent them from being
   // deleted by optimizations.
   void initializeLibCalls(const Module &TheModule) {
     TargetLibraryInfoImpl TLII(Triple(TM.getTargetTriple()));
     TargetLibraryInfo TLI(TLII);
 
for (const Function &F : TheModule) {
         for (unsigned I = 0, E = static_cast<unsigned>(RTLIB::UNKNOWN_LIBCALL);
              I != E; ++I)
           if (const char *Name =
                   Lowering->getLibcallName(static_cast<RTLIB::Libcall>(I)))
             Libcalls.insert(Name);
     }
   }
 
-  void findLibCallsAndAsm(const GlobalValue &GV) {
+  void findLibCallsAndAsm(GlobalValue &GV) {
     // There are no restrictions to apply to declarations.
     if (GV.isDeclaration())
       return;
 
     // There is nothing more restrictive than private linkage.
     if (GV.hasPrivateLinkage())
       return;
 
     // Conservatively append user-supplied runtime library functions to
     // llvm.compiler.used.  These could be internalized and deleted by
     // optimizations like -globalopt, causing problems when later optimizations
     // add new library calls (e.g., llvm.memset => memset and printf => puts).
     // Leave it to the linker to remove any dead code (e.g. with -dead_strip).
-    if (isa<Function>(GV) && Libcalls.count(GV.getName()))
-      LLVMUsed.insert(&GV);
+    if (isa<Function>(GV) && Libcalls.count(GV.getName())) {
+      LLVMUsed.push_back(&GV);
+      return;
+    }
 
     SmallString<64> Buffer;
     TM.getNameWithPrefix(Buffer, &GV, Mangler);
     if (AsmUndefinedRefs.count(Buffer))
-      LLVMUsed.insert(&GV);
+      LLVMUsed.push_back(&GV);
   }
 };
 
 } // namespace anonymous
 
 void llvm::updateCompilerUsed(Module &TheModule, const TargetMachine &TM,
                               const StringSet<> &AsmUndefinedRefs) {
-  SmallPtrSet<const GlobalValue *, 8> UsedValues;
+  std::vector<GlobalValue *> UsedValues;
   PreserveLibCallsAndAsmUsed(AsmUndefinedRefs, TM, UsedValues)
       .findInModule(TheModule);
 
   if (UsedValues.empty())
     return;
 
-  llvm::Type *i8PTy = llvm::Type::getInt8PtrTy(TheModule.getContext());
-  std::vector<Constant *> UsedValuesList;
-  for (const auto *GV : UsedValues) {
-    Constant *c =
-        ConstantExpr::getBitCast(const_cast<GlobalValue *>(GV), i8PTy);
-    UsedValuesList.push_back(c);
-  }
-
-  GlobalVariable *LLVMUsed = TheModule.getGlobalVariable("llvm.compiler.used");
-  if (LLVMUsed) {
-    ConstantArray *Inits = cast<ConstantArray>(LLVMUsed->getInitializer());
-    for (auto &V : Inits->operands())
-      UsedValuesList.push_back(cast<Constant>(&V));
-    LLVMUsed->eraseFromParent();
-  }
-
-  llvm::ArrayType *ATy = llvm::ArrayType::get(i8PTy, UsedValuesList.size());
-  LLVMUsed = new llvm::GlobalVariable(
-      TheModule, ATy, false, llvm::GlobalValue::AppendingLinkage,
-      llvm::ConstantArray::get(ATy, UsedValuesList), "llvm.compiler.used");
-
-  LLVMUsed->setSection("llvm.metadata");
+  appendToCompilerUsed(TheModule, UsedValues);
 }

lib/Transforms/Instrumentation/AddressSanitizer.cpp

RegisteredFlag = new GlobalVariable(
         M, IntptrTy, false, GlobalVariable::CommonLinkage,
         ConstantInt::get(IntptrTy, 0), kAsanGlobalsRegisteredFlagName);
 
     // We also emit a structure which binds the liveness of the global
     // variable to the metadata struct.
     StructType *LivenessTy = StructType::get(IntptrTy, IntptrTy, nullptr);
 
     // Keep the list of "Liveness" GV created to be added to llvm.compiler.used
-    SmallVector<Constant *, 16> LivenessGlobals;
+    SmallVector<GlobalValue *, 16> LivenessGlobals;
     LivenessGlobals.reserve(n);
 
     for (size_t i = 0; i < n; i++) {
       GlobalVariable *Metadata = new GlobalVariable(
           M, GlobalStructTy, false, GlobalVariable::InternalLinkage,
           Initializers[i], "");
       Metadata->setSection("__DATA,__asan_globals,regular");
       Metadata->setAlignment(1); // don't leave padding in between
 
       auto LivenessBinder = ConstantStruct::get(LivenessTy,
           Initializers[i]->getAggregateElement(0u),
           ConstantExpr::getPointerCast(Metadata, IntptrTy),
           nullptr);
 
       // Recover the name of the variable this global is pointing to
       StringRef GVName =
           Initializers[i]->getAggregateElement(0u)->getOperand(0)->getName();
 
       GlobalVariable *Liveness = new GlobalVariable(
           M, LivenessTy, false, GlobalVariable::InternalLinkage, LivenessBinder,
           Twine("__asan_binder_") + GVName);
       Liveness->setSection("__DATA,__asan_liveness,regular,live_support");
-      LivenessGlobals.push_back(
-          ConstantExpr::getBitCast(Liveness, IRB.getInt8PtrTy()));
+      LivenessGlobals.push_back(Liveness);
     }
 
-    if (!LivenessGlobals.empty()) {
-      // Update llvm.compiler.used, adding the new liveness globals. This is
-      // needed so that during LTO these variables stay alive. The alternative
-      // would be to have the linker handling the LTO symbols, but libLTO
-      // current
-      // API does not expose access to the section for each symbol.
-      if (GlobalVariable *LLVMUsed =
-              M.getGlobalVariable("llvm.compiler.used")) {
-        ConstantArray *Inits = cast<ConstantArray>(LLVMUsed->getInitializer());
-        for (auto &V : Inits->operands())
-          LivenessGlobals.push_back(cast<Constant>(&V));
-        LLVMUsed->eraseFromParent();
-      }
-      llvm::ArrayType *ATy =
-          llvm::ArrayType::get(IRB.getInt8PtrTy(), LivenessGlobals.size());
-      auto *LLVMUsed = new llvm::GlobalVariable(
-          M, ATy, false, llvm::GlobalValue::AppendingLinkage,
-          llvm::ConstantArray::get(ATy, LivenessGlobals), "llvm.compiler.used");
-      LLVMUsed->setSection("llvm.metadata");
-    }
+    // current API does not expose access to the section for each symbol.
+    if (!LivenessGlobals.empty())
+      appendToCompilerUsed(M, LivenessGlobals);
   } else {
     // On all other platfoms, we just emit an array of global metadata
     // structures.
     ArrayType *ArrayOfGlobalStructTy = ArrayType::get(GlobalStructTy, n);
     AllGlobals = new GlobalVariable(
         M, ArrayOfGlobalStructTy, false, GlobalVariable::InternalLinkage,
         ConstantArray::get(ArrayOfGlobalStructTy, Initializers), "");
   }

lib/Transforms/Instrumentation/InstrProfiling.cpp

void InstrProfiling::emitRuntimeHook() {
   auto *Load = IRB.CreateLoad(Var);
   IRB.CreateRet(Load);
 
   // Mark the user variable as used so that it isn't stripped out.
   UsedVars.push_back(User);
 }
 
 void InstrProfiling::emitUses() {
-  if (UsedVars.empty())
-    return;
-
-  GlobalVariable *LLVMUsed = M->getGlobalVariable("llvm.used");
-  std::vector<Constant *> MergedVars;
-  if (LLVMUsed) {
-    // Collect the existing members of llvm.used.
-    ConstantArray *Inits = cast<ConstantArray>(LLVMUsed->getInitializer());
-    for (unsigned I = 0, E = Inits->getNumOperands(); I != E; ++I)
-      MergedVars.push_back(Inits->getOperand(I));
-    LLVMUsed->eraseFromParent();
-  }
-
-  Type *i8PTy = Type::getInt8PtrTy(M->getContext());
-  // Add uses for our data.
-  for (auto *Value : UsedVars)
-    MergedVars.push_back(
-        ConstantExpr::getBitCast(cast<Constant>(Value), i8PTy));
-
-  // Recreate llvm.used.
-  ArrayType *ATy = ArrayType::get(i8PTy, MergedVars.size());
-  LLVMUsed =
-      new GlobalVariable(*M, ATy, false, GlobalValue::AppendingLinkage,
-                         ConstantArray::get(ATy, MergedVars), "llvm.used");
-  LLVMUsed->setSection("llvm.metadata");
+  if (!UsedVars.empty())
+    appendToUsed(*M, UsedVars);
 }
 
 void InstrProfiling::emitInitialization() {
   StringRef InstrProfileOutput = Options.InstrProfileOutput;
 
   if (!InstrProfileOutput.empty()) {
     // Create variable for profile name.
     Constant *ProfileNameConst =

lib/Transforms/Utils/ModuleUtils.cpp

 void llvm::appendToGlobalCtors(Module &M, Function *F, int Priority, Constant *Data) {
   appendToGlobalArray("llvm.global_ctors", M, F, Priority, Data);
 }
 
 void llvm::appendToGlobalDtors(Module &M, Function *F, int Priority, Constant *Data) {
   appendToGlobalArray("llvm.global_dtors", M, F, Priority, Data);
 }
 
+static void appendToUsedList(Module &M, StringRef Name, ArrayRef<GlobalValue *> Values) {
+  GlobalVariable *GV = M.getGlobalVariable(Name);
+  SmallPtrSet<Constant *, 16> InitAsSet;
+  SmallVector<Constant *, 16> Init;
+  if (GV) {
+    ConstantArray *CA = dyn_cast<ConstantArray>(GV->getInitializer());
+    for (auto &Op : CA->operands()) {
+      Constant *C = cast_or_null<Constant>(Op);
+      if (InitAsSet.insert(C).second)
+        Init.push_back(C);
+    }
+    GV->eraseFromParent();
+  }
+
+  Type *Int8PtrTy = llvm::Type::getInt8PtrTy(M.getContext());
+  for (auto *V : Values) {
+    Constant *C = ConstantExpr::getBitCast(V, Int8PtrTy);
+    if (InitAsSet.insert(C).second)
+      Init.push_back(C);
+  }
+
+  if (Init.empty())
+    return;
+
+  ArrayType *ATy = ArrayType::get(Int8PtrTy, Init.size());
+  GV = new llvm::GlobalVariable(M, ATy, false, GlobalValue::AppendingLinkage,
+                                ConstantArray::get(ATy, Init), Name);
+  GV->setSection("llvm.metadata");
+}
+
+void llvm::appendToUsed(Module &M, ArrayRef<GlobalValue *> Values) {
+  appendToUsedList(M, "llvm.used", Values);
+}
+
+void llvm::appendToCompilerUsed(Module &M, ArrayRef<GlobalValue *> Values) {
+  appendToUsedList(M, "llvm.compiler.used", Values);
+}
+
 Function *llvm::checkSanitizerInterfaceFunction(Constant *FuncOrBitcast) {
   if (isa<Function>(FuncOrBitcast))
     return cast<Function>(FuncOrBitcast);
   FuncOrBitcast->dump();
   std::string Err;
   raw_string_ostream Stream(Err);
   Stream << "Sanitizer interface function redefined: " << *FuncOrBitcast;
   report_fatal_error(Err);