[PATCH] D26063: [ThinLTO] Use NoPromote flag in summary during promotion

[Mehdi AMINI via llvm-commits]

mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: include/llvm/Transforms/Utils/FunctionImportUtils.h:44
   /// Check if we should promote the given local value to global scope.
   bool doPromoteLocalToGlobal(const GlobalValue *SGV);
 
----------------
The name should be "shouldPromoteLocalToGlobal", the "do" added some cognitive load for me to read this patch (I'm not saying that it should be done as part of this patch, but either before or after would be a nice tweak).


================
Comment at: lib/Transforms/Utils/FunctionImportUtils.cpp:70
 
-  if (GVar && GVar->hasSection())
-    // Some sections like "__DATA,__cfstring" are "magic" and promotion is not
-    // allowed. Just disable promotion on any GVar with sections right now.
+  auto *Summary = ImportIndex.getGlobalValueSummary(SGV->getGUID());
+  assert(Summary && "Missing summary for global value");
----------------
This `SGV->getGUID()` is the cause for the "DoPromote" right?


https://reviews.llvm.org/D26063