[ThinLTO] Make ValueInfo::operator bool() explicit

[evgeny777]

Summary

This prevents two bad things from happening:

• implicit conversion to GUID
• comparison (VI1 == VI2) based on implicit bool conversion

This change immediately uncovered bug in stats printing, so I fixed it as well

[steven_wu]

LGTM

[tejohnson]

llvm/lib/LTO/LTO.cpp

426

Is this a bug fix exposed by your change or just unrelated cleanup? If the latter please commit separately.

llvm/include/llvm/IR/ModuleSummaryIndex.h

PointerIntPair<const GlobalValueSummaryMapTy::value_type *, 3, int>
       RefAndFlags;
 
   ValueInfo() = default;
   ValueInfo(bool HaveGVs, const GlobalValueSummaryMapTy::value_type *R) {
     RefAndFlags.setPointer(R);
     RefAndFlags.setInt(HaveGVs);
   }
 
-  operator bool() const { return getRef(); }
+  explicit operator bool() const { return getRef(); }
 
   GlobalValue::GUID getGUID() const { return getRef()->first; }
   const GlobalValue *getValue() const {
     assert(haveGVs());
     return getRef()->second.U.GV;
   }
 
   ArrayRef<std::unique_ptr<GlobalValueSummary>> getSummaryList() const {

llvm/lib/LTO/LTO.cpp

static bool isWeakObjectWithRWAccess(GlobalValueSummary *GVS) {
   if (auto *VarSummary = dyn_cast<GlobalVarSummary>(GVS->getBaseObject()))
     return !VarSummary->maybeReadOnly() && !VarSummary->maybeWriteOnly() &&
            (VarSummary->linkage() == GlobalValue::WeakODRLinkage ||
             VarSummary->linkage() == GlobalValue::LinkOnceODRLinkage);
   return false;
 }
 
 static void thinLTOInternalizeAndPromoteGUID(
-    GlobalValueSummaryList &GVSummaryList, ValueInfo VI,
-    function_ref<bool(StringRef, ValueInfo)> isExported,
+    ValueInfo VI, function_ref<bool(StringRef, ValueInfo)> isExported,
     function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
         isPrevailing) {
-  for (auto &S : GVSummaryList) {
+  for (auto &S : VI.getSummaryList()) {
     if (isExported(S->modulePath(), VI)) {
       if (GlobalValue::isLocalLinkage(S->linkage()))
         S->setLinkage(GlobalValue::ExternalLinkage);
     } else if (EnableLTOInternalization &&
                // Ignore local and appending linkage values since the linker
                // doesn't resolve them.
                !GlobalValue::isLocalLinkage(S->linkage()) &&
                (!GlobalValue::isInterposableLinkage(S->linkage()) ||

 // Update the linkages in the given \p Index to mark exported values
 // as external and non-exported values as internal.
 void llvm::thinLTOInternalizeAndPromoteInIndex(
     ModuleSummaryIndex &Index,
     function_ref<bool(StringRef, ValueInfo)> isExported,
     function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
         isPrevailing) {
   for (auto &I : Index)
-    thinLTOInternalizeAndPromoteGUID(
-        I.second.SummaryList, Index.getValueInfo(I), isExported, isPrevailing);
+    thinLTOInternalizeAndPromoteGUID(Index.getValueInfo(I), isExported,
+                                     isPrevailing);
 }
 
 // Requires a destructor for std::vector<InputModule>.
 InputFile::~InputFile() = default;
 
 Expected<std::unique_ptr<InputFile>> InputFile::create(MemoryBufferRef Object) {
   std::unique_ptr<InputFile> File(new InputFile);
 

llvm/lib/Transforms/IPO/FunctionImport.cpp

for (auto &I : ImportThresholds) {
              << ", Size = " << (FS ? (int)FS->instCount() : -1)
              << ", MaxHotness = " << getHotnessName(FailureInfo->MaxHotness)
              << ", Attempts = " << FailureInfo->Attempts << "\n";
     }
   }
 }
 
 #ifndef NDEBUG
-static bool isGlobalVarSummary(const ModuleSummaryIndex &Index,
-                               GlobalValue::GUID G) {
-  if (const auto &VI = Index.getValueInfo(G)) {
-    auto SL = VI.getSummaryList();
-    if (!SL.empty())
-      return SL[0]->getSummaryKind() == GlobalValueSummary::GlobalVarKind;
-  }
+static bool isGlobalVarSummary(const ModuleSummaryIndex &Index, ValueInfo VI) {
+  return SL.empty()
+             ? false
+             : SL[0]->getSummaryKind() == GlobalValueSummary::GlobalVarKind;
+
+static bool isGlobalVarSummary(const ModuleSummaryIndex &Index,
+                               GlobalValue::GUID G) {
+  if (const auto &VI = Index.getValueInfo(G))
+    return isGlobalVarSummary(Index, VI);
   return false;
 }
 
-static GlobalValue::GUID getGUID(GlobalValue::GUID G) { return G; }
-
 template <class T>
 static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index,
                                       T &Cont) {
   unsigned NumGVS = 0;
   for (auto &V : Cont)
-    if (isGlobalVarSummary(Index, getGUID(V)))
+    if (isGlobalVarSummary(Index, V))
       ++NumGVS;
   return NumGVS;
 }
 #endif
 
 static bool
 checkVariableImport(const ModuleSummaryIndex &Index,
                     StringMap<FunctionImporter::ImportMapTy> &ImportLists,

llvm/test/ThinLTO/X86/funcimport-stats.ll

+; REQUIRES: asserts
+; RUN: opt -module-summary %p/funcimport.ll -o %t.bc
+; RUN: opt -module-summary %p/Inputs/funcimport.ll -o %t2.bc
+; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t.bc %t2.bc
+
+; RUN: llvm-lto -thinlto-action=import %t2.bc -thinlto-index=%t3.bc -o /dev/null -debug-only=function-import -stats > %t4 2>&1
+; RUN: cat %t4 | grep 'Is importing global' | count 4
+; RUN: cat %t4 | grep 'Is importing function' | count 8
+; RUN: cat %t4 | grep 'Is importing aliasee' | count 1
+; RUN: cat %t4 | FileCheck %s
+
+; CHECK:      - [[NUM_FUNCS:[0-9]+]] functions imported from
+; CHECK-NEXT: - [[NUM_VARS:[0-9]+]] global vars imported from
+
+; CHECK:      [[NUM_FUNCS]] function-import - Number of functions imported in backend
+; CHECK-NEXT: [[NUM_FUNCS]] function-import - Number of functions thin link decided to import
+; CHECK-NEXT: [[NUM_VARS]] function-import - Number of global variables imported in backend
+; CHECK-NEXT: [[NUM_VARS]] function-import - Number of global variables thin link decided to import
+; CHECK-NEXT: 1 function-import - Number of modules imported from
+; CHECK-NEXT: [[NUM_VARS]] module-summary-index - Number of live global variables marked read only
+; CHECK-NEXT: 1 module-summary-index - Number of live global variables marked write only