ModuleSummaryAnalysis: Correctly handle refs from function inline asm to module inline asm.

[pcc]

Summary

If a function contains inline asm and the module-level inline asm
contains the definition of a local symbol, prevent the function from
being imported in case the function-level inline asm refers to a
symbol in the module-level inline asm.

[mehdi_amini]

llvm/test/Bitcode/thinlto-asm-noimport.ll

2 ↗

(On Diff #113506)

Is llvm-bcanalyzer *still* our only / preferred way of testing? :(

5 ↗

(On Diff #113506)

I suspect this test is target dependent (requires the backend compiled in to parse the inline asm) and so couldn't be in the generic bitcode test.

[mehdi_amini]

LGTM otherwise.

[tejohnson]

LGTM too other than test target dependent issue Mehdi pointed out.

llvm/test/Bitcode/thinlto-asm-noimport.ll

2 ↗

(On Diff #113506)

Sadly, yes. =(

[pcc]

llvm/test/Bitcode/thinlto-asm-noimport.ll

5 ↗

(On Diff #113506)

Added a REQUIRES line.

llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp

static void addIntrinsicToSummary(
   default:
     break;
   }
 }
 
 static void
 computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
                        const Function &F, BlockFrequencyInfo *BFI,
-                       ProfileSummaryInfo *PSI, bool HasLocalsInUsed,
+                       ProfileSummaryInfo *PSI, bool HasLocalsInUsedOrAsm,
                        DenseSet<GlobalValue::GUID> &CantBePromoted) {
   // Summary not currently supported for anonymous functions, they should
   // have been named.
   assert(F.hasName());
 
   unsigned NumInsts = 0;
   // Map from callee ValueId to profile count. Used to accumulate profile
   // counts for all static calls to a given callee.
for (const Instruction &I : BB) {
         continue;
 
       const auto *CI = dyn_cast<CallInst>(&I);
       // Since we don't know exactly which local values are referenced in inline
       // assembly, conservatively mark the function as possibly referencing
       // a local value from inline assembly to ensure we don't export a
       // reference (which would require renaming and promotion of the
       // referenced value).
-      if (HasLocalsInUsed && CI && CI->isInlineAsm())
+      if (HasLocalsInUsedOrAsm && CI && CI->isInlineAsm())
         HasInlineAsmMaybeReferencingInternal = true;
 
       auto *CalledValue = CS.getCalledValue();
       auto *CalledFunction = CS.getCalledFunction();
       // Check if this is an alias to a function. If so, get the
       // called aliasee for the checks below.
       if (auto *GA = dyn_cast<GlobalAlias>(CalledValue)) {
         assert(!CalledFunction && "Expected null called function in callsite for alias");
ModuleSummaryIndex llvm::buildModuleSummaryIndex(
   DenseSet<GlobalValue::GUID> CantBePromoted;
   for (auto *V : Used) {
     if (V->hasLocalLinkage()) {
       LocalsUsed.insert(V);
       CantBePromoted.insert(V->getGUID());
     }
   }
 
+  bool HasLocalInlineAsmSymbol = false;
+  if (!M.getModuleInlineAsm().empty()) {
+    // Collect the local values defined by module level asm, and set up
+    // summaries for these symbols so that they can be marked as NoRename,
+    // to prevent export of any use of them in regular IR that would require
+    // renaming within the module level asm. Note we don't need to create a
+    // summary for weak or global defs, as they don't need to be flagged as
+    // NoRename, and defs in module level asm can't be imported anyway.
+    // Also, any values used but not defined within module level asm should
+    // be listed on the llvm.used or llvm.compiler.used global and marked as
+    // referenced from there.
+    ModuleSymbolTable::CollectAsmSymbols(
+        M, [&](StringRef Name, object::BasicSymbolRef::Flags Flags) {
+          // Symbols not marked as Weak or Global are local definitions.
+          if (Flags & (object::BasicSymbolRef::SF_Weak |
+                       object::BasicSymbolRef::SF_Global))
+            return;
+          HasLocalInlineAsmSymbol = true;
+          GlobalValue *GV = M.getNamedValue(Name);
+          if (!GV)
+            return;
+          assert(GV->isDeclaration() && "Def in module asm already has definition");
+          GlobalValueSummary::GVFlags GVFlags(GlobalValue::InternalLinkage,
+                                              /* NotEligibleToImport = */ true,
+                                              /* Live = */ true);
+          CantBePromoted.insert(GlobalValue::getGUID(Name));
+          // Create the appropriate summary type.
+          if (Function *F = dyn_cast<Function>(GV)) {
+            std::unique_ptr<FunctionSummary> Summary =
+                llvm::make_unique<FunctionSummary>(
+                    GVFlags, 0,
+                    FunctionSummary::FFlags{
+                        F->hasFnAttribute(Attribute::ReadNone),
+                        F->hasFnAttribute(Attribute::ReadOnly),
+                        F->hasFnAttribute(Attribute::NoRecurse),
+                        F->returnDoesNotAlias()},
+                    ArrayRef<ValueInfo>{}, ArrayRef<FunctionSummary::EdgeTy>{},
+                    ArrayRef<GlobalValue::GUID>{},
+                    ArrayRef<FunctionSummary::VFuncId>{},
+                    ArrayRef<FunctionSummary::VFuncId>{},
+                    ArrayRef<FunctionSummary::ConstVCall>{},
+                    ArrayRef<FunctionSummary::ConstVCall>{});
+            Index.addGlobalValueSummary(Name, std::move(Summary));
+          } else {
+            std::unique_ptr<GlobalVarSummary> Summary =
+                llvm::make_unique<GlobalVarSummary>(GVFlags,
+                                                    ArrayRef<ValueInfo>{});
+            Index.addGlobalValueSummary(Name, std::move(Summary));
+          }
+        });
+  }
+
   // Compute summaries for all functions defined in module, and save in the
   // index.
   for (auto &F : M) {
     if (F.isDeclaration())
       continue;
 
     BlockFrequencyInfo *BFI = nullptr;
     std::unique_ptr<BlockFrequencyInfo> BFIPtr;
     if (GetBFICallback)
       BFI = GetBFICallback(F);
     else if (F.getEntryCount().hasValue()) {
       LoopInfo LI{DominatorTree(const_cast<Function &>(F))};
       BranchProbabilityInfo BPI{F, LI};
       BFIPtr = llvm::make_unique<BlockFrequencyInfo>(F, BPI, LI);
       BFI = BFIPtr.get();
     }
 
-    computeFunctionSummary(Index, M, F, BFI, PSI, !LocalsUsed.empty(),
+    computeFunctionSummary(Index, M, F, BFI, PSI,
+                           !LocalsUsed.empty() || HasLocalInlineAsmSymbol,
                            CantBePromoted);
   }
 
   // Set live flag for all personality functions. That allows to
   // preserve them during DCE.
   for (const llvm::Function &F : M)
     if (!F.isDeclaration() && F.hasPersonalityFn())
       setLiveRoot(Index, F.getPersonalityFn()->getName());
ModuleSummaryIndex llvm::buildModuleSummaryIndex(
   // to flag them as live in the index to ensure index-based dead value
   // analysis treats them as live roots of the analysis.
   setLiveRoot(Index, "llvm.used");
   setLiveRoot(Index, "llvm.compiler.used");
   setLiveRoot(Index, "llvm.global_ctors");
   setLiveRoot(Index, "llvm.global_dtors");
   setLiveRoot(Index, "llvm.global.annotations");
 
-  if (!M.getModuleInlineAsm().empty()) {
-    // Collect the local values defined by module level asm, and set up
-    // summaries for these symbols so that they can be marked as NoRename,
-    // to prevent export of any use of them in regular IR that would require
-    // renaming within the module level asm. Note we don't need to create a
-    // summary for weak or global defs, as they don't need to be flagged as
-    // NoRename, and defs in module level asm can't be imported anyway.
-    // Also, any values used but not defined within module level asm should
-    // be listed on the llvm.used or llvm.compiler.used global and marked as
-    // referenced from there.
-    ModuleSymbolTable::CollectAsmSymbols(
-        M, [&M, &Index, &CantBePromoted](StringRef Name,
-                                         object::BasicSymbolRef::Flags Flags) {
-          // Symbols not marked as Weak or Global are local definitions.
-          if (Flags & (object::BasicSymbolRef::SF_Weak |
-                       object::BasicSymbolRef::SF_Global))
-            return;
-          GlobalValue *GV = M.getNamedValue(Name);
-          if (!GV)
-            return;
-          assert(GV->isDeclaration() && "Def in module asm already has definition");
-          GlobalValueSummary::GVFlags GVFlags(GlobalValue::InternalLinkage,
-                                              /* NotEligibleToImport = */ true,
-                                              /* Live = */ true);
-          CantBePromoted.insert(GlobalValue::getGUID(Name));
-          // Create the appropriate summary type.
-          if (Function *F = dyn_cast<Function>(GV)) {
-            std::unique_ptr<FunctionSummary> Summary =
-                llvm::make_unique<FunctionSummary>(
-                    GVFlags, 0,
-                    FunctionSummary::FFlags{
-                        F->hasFnAttribute(Attribute::ReadNone),
-                        F->hasFnAttribute(Attribute::ReadOnly),
-                        F->hasFnAttribute(Attribute::NoRecurse),
-                        F->returnDoesNotAlias()},
-                    ArrayRef<ValueInfo>{}, ArrayRef<FunctionSummary::EdgeTy>{},
-                    ArrayRef<GlobalValue::GUID>{},
-                    ArrayRef<FunctionSummary::VFuncId>{},
-                    ArrayRef<FunctionSummary::VFuncId>{},
-                    ArrayRef<FunctionSummary::ConstVCall>{},
-                    ArrayRef<FunctionSummary::ConstVCall>{});
-            Index.addGlobalValueSummary(Name, std::move(Summary));
-          } else {
-            std::unique_ptr<GlobalVarSummary> Summary =
-                llvm::make_unique<GlobalVarSummary>(GVFlags,
-                                                    ArrayRef<ValueInfo>{});
-            Index.addGlobalValueSummary(Name, std::move(Summary));
-          }
-        });
-  }
-
   bool IsThinLTO = true;
   if (auto *MD =
           mdconst::extract_or_null<ConstantInt>(M.getModuleFlag("ThinLTO")))
     IsThinLTO = MD->getZExtValue();
 
   for (auto &GlobalList : Index) {
     // Ignore entries for references that are undefined in the current module.
     if (GlobalList.second.SummaryList.empty())

llvm/trunk/test/Bitcode/thinlto-asm-noimport.ll

+; REQUIRES: arm-registered-target
+; RUN: opt -module-summary -o %t %s
+; RUN: llvm-bcanalyzer -dump %t | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "thumbv7--linux-android"
+
+module asm "asm:"
+module asm "bx lr"
+
+; NotEligibleToImport
+; CHECK: <PERMODULE {{.*}} op1=16
+define void @f() {
+  call void asm sideeffect "bl asm\0A", ""()
+  ret void
+}