[PATCH] D26146: [ThinLTO] Prevent exporting of locals used/defined in module level asm

[Mehdi AMINI via llvm-commits]

mehdi_amini added a comment.



> Then the assumption works for this fix since it is only looking for static globals (i.e. local linkage), right? I can just update the description and possibly the code comment if that is the case

The `attribute((used))` will makes use of the `@llvm.used`, which you already handled before this patch right?



================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:426
+                                            bool PerModuleIndex = true,
+                                            bool AssertExists = true) const {
     assert(GV.hasName() && "Can't get GlobalValueSummary for GV with no name");
----------------
I'm not convinced by this API: the extra parameter corresponds exactly to what is usually exposed as a separate API (like `std::vector::get` vs `std::vector::operator[]` for example)



================
Comment at: lib/Analysis/ModuleSummaryAnalysis.cpp:208
       LocalsUsed.insert(V);
   }
 
----------------
Can you clarify what is specific here that wouldn't make us add here the "compiler_used" variables?


================
Comment at: lib/Analysis/ModuleSummaryAnalysis.cpp:243
 
+  // Collect the set of llvm.compiler.used locak variables, which along with the
+  // llvm.used variables already in the LocalUsed set, cannot be renamed as
----------------
s/locak/local/


================
Comment at: lib/Analysis/ModuleSummaryAnalysis.cpp:276
+          if (!(Flags & (object::BasicSymbolRef::SF_Weak ||
+                         object::BasicSymbolRef::SF_Global))) {
+            GlobalValue *GV = M.getNamedValue(Name);
----------------
Early exit


https://reviews.llvm.org/D26146