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

[tejohnson]

Summary

This patch uses the same approach added for inline asm in r285513 to
similarly prevent promotion/renaming of locals used or defined in module
level asm.

All global values defined in normal IR and used in module level asm
should be included on either the llvm.used or llvm.compiler.used global.
The former were already being flagged as NoRename in the summary, and
I've simply added llvm.compiler.used values to this handling.

Module level asm may also contain defs of values. We need to prevent
export of any refs to local values defined in module level asm (e.g. a
ref in normal IR), since that also requires renaming/promotion of the
local. To do that, the summary index builder looks at all values in the
module level asm string that are not marked Weak or Global, which is
exactly the set of locals that are defined. A summary is created for
each of these local defs and flagged as NoRename.

This required adding handling to the BitcodeWriter to look at GV
declarations to see if they have a summary (rather than skipping them
all).

Finally, added an assert to IRObjectFile::CollectAsmUndefinedRefs to
ensure that an MCAsmParser is available, otherwise the module asm parse
would silently fail. Initialized the asm parser in the opt tool for use
in testing this fix.

Fixes PR30610.

[mehdi_amini]

All global values defined in normal IR and used in module level asm should be included on either the llvm.used or llvm.compiler.used global.

Can you clarify where this requirement comes from?

[tejohnson]

In D26146#585130, @mehdi_amini wrote:

All global values defined in normal IR and used in module level asm should be included on either the llvm.used or llvm.compiler.used global.

Can you clarify where this requirement comes from?

For inline asm, according to http://llvm.org/docs/LangRef.html#the-llvm-used-global-variable:
"This is commonly used to represent references from inline asms and other things the compiler cannot “see”, and corresponds to “attribute((used))” in GNU C."

And for module level asm, I got this info from you =). It was via the conversation we had on the Chromium build issue that discussed this fix:

On Thu, Oct 27, 2016 at 8:08 AM, Mehdi Amini <[e-mail]> wrote:

On Oct 27, 2016, at 6:51 AM, Teresa Johnson <[e-mail]> wrote:

On Tue, Oct 25, 2016 at 8:31 PM, Mehdi Amini <[e-mail]> wrote:

On Oct 25, 2016, at 11:06 AM, Teresa Johnson <[e-mail]> wrote:

I have a draft set of fixes in https://reviews.llvm.org/D25951 (WIP, not for review). I've done some testing using the regression tests and one I added for module asm, but it needs cleanup and more testing. Feel free to give it a try.

I'm going to split it up and send various parts for review separately - it was a bit more involved than I expected.

Also, I have attempted to handle the case where a local value is used in module level asm. E.g.:

---

@b = internal global i32 1, align 4

module asm "\09.text"
module asm "\09.type\09foo,@function"
module asm "foo:"
module asm "\09movl b, %eax"
module asm "\09ret "
module asm "\09.size\09foo, .-foo"
module asm “"

The frontend is responsible to add @b to llvm.compiler_used.

Ok, adding the llvm.compiler.used containing @b fixes the opt -O2 build of this case. This will simplify the patch as I don't need to look for uses within module asm blocks (just defs). I can simply flag all values within both llvm.used and llvm.compiler.used as non-renamable.

For inline asm we assume the uses are added to llvm.used, not llvm.compiler.used - I guess this is because they are user inserted? I do see in CGObjCMac.cpp where there are a bunch of calls to add to the compiler used, but that seems to be when CGObjCMac is synthesizing module assembly. In the Chromium case the module level asm is generated from file scope asm() calls - I assume it is therefore the user's responsibility to mark any uses within those with “attribute((used))”, just as one would for inline asm (asm added within a function), right?

Right.

—
Mehdi

[mehdi_amini]

The attribute((used)) goes beyond "what the compiler can see", it also prevents linker dead-strip.

The point for references from inline ASM only applies to static globals (chromium build). It does not apply to non-static I believe, and this is why are currently parsing the module inline ASM at LTO time and updating the llvm.compiler_used, because this is not something the FE is required to do.

[tejohnson]

In D26146#585158, @mehdi_amini wrote:

The attribute((used)) goes beyond "what the compiler can see", it also prevents linker dead-strip.

The point for references from inline ASM only applies to static globals (chromium build). It does not apply to non-static I believe, and this is why are currently parsing the module inline ASM at LTO time and updating the llvm.compiler_used, because this is not something the FE is required to do.

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

[mehdi_amini]

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?

include/llvm/IR/ModuleSummaryIndex.h

426

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)

lib/Analysis/ModuleSummaryAnalysis.cpp

208

Can you clarify what is specific here that wouldn't make us add here the "compiler_used" variables?

243

s/locak/local/

276

Early exit

[tejohnson]

In D26146#585633, @mehdi_amini wrote:

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?

Right for llvm.used, but I was referring to static (local linkage) values in the llvm.compiler.used which is added in this patch, since you mentioned: "It does not apply to non-static I believe, and this is why are currently parsing the module inline ASM at LTO time and updating the llvm.compiler_used, because this is not something the FE is required to do."

I read from that that any statics would have been placed by the FE in the llvm.compiler.used set, or is that not a correct interpretation? Going back to the email discussion I included in my earlier response, it seems that it must be true that "The frontend is responsible to add [local ref] @b to llvm.compiler_used", in which case the handling in this current patch that looks for locals in the llvm.compiler.used is correct, right?

include/llvm/IR/ModuleSummaryIndex.h

426

I removed this change and modified the use sites to instead use the existing findGlobalValueSummaryList.

lib/Analysis/ModuleSummaryAnalysis.cpp

208

I updated the comment above this block to address this hopefully.

243

fixed

276

fixed

[tejohnson]

Addressed review comments.

[mehdi_amini]

In D26146#589341, @tejohnson wrote:

In D26146#585633, @mehdi_amini wrote:

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?

Right for llvm.used, but I was referring to static (local linkage) values in the llvm.compiler.used which is added in this patch, since you mentioned: "It does not apply to non-static I believe, and this is why are currently parsing the module inline ASM at LTO time and updating the llvm.compiler_used, because this is not something the FE is required to do."

I read from that that any statics would have been placed by the FE in the llvm.compiler.used set, or is that not a correct interpretation? Going back to the email discussion I included in my earlier response, it seems that it must be true that "The frontend is responsible to add [local ref] @b to llvm.compiler_used", in which case the handling in this current patch that looks for locals in the llvm.compiler.used is correct, right?

In D26146#589341, @tejohnson wrote:

In D26146#585633, @mehdi_amini wrote:

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?

Right for llvm.used, but I was referring to static (local linkage) values in the llvm.compiler.used which is added in this patch, since you mentioned: "It does not apply to non-static I believe, and this is why are currently parsing the module inline ASM at LTO time and updating the llvm.compiler_used, because this is not something the FE is required to do."

I read from that that any statics would have been placed by the FE in the llvm.compiler.used set, or is that not a correct interpretation? Going back to the email discussion I included in my earlier response, it seems that it must be true that "The frontend is responsible to add [local ref] @b to llvm.compiler_used", in which case the handling in this current patch that looks for locals in the llvm.compiler.used is correct, right?

To be more precise: the FE can put uses from inline ASM to static symbol in either llvm.used or llvm.compiler_used (i.e. the latter should be "enough").
(for clang, this is done by the user who usually in C++ uses __attribute__((used)), which translates into llvm.used)

lib/Analysis/ModuleSummaryAnalysis.cpp

208

My question was the opposite: why *only* llvm.used here, and not *also* llvm.compiler_used.

You wrote here "We collect them here because ...." and the same reasons should apply to "compiler_used".

252

To be more clear with the above, this is the snippet I asked about why is it done here and not above around line 212.

[mehdi_amini]

test/ThinLTO/X86/module_asm2.ll

74

Can you add a comment to this file explaining what you're testing, maybe with each of the CHECK groups.

[tejohnson]

In D26146#589364, @mehdi_amini wrote:

In D26146#589341, @tejohnson wrote:

In D26146#585633, @mehdi_amini wrote:

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?

Right for llvm.used, but I was referring to static (local linkage) values in the llvm.compiler.used which is added in this patch, since you mentioned: "It does not apply to non-static I believe, and this is why are currently parsing the module inline ASM at LTO time and updating the llvm.compiler_used, because this is not something the FE is required to do."

I read from that that any statics would have been placed by the FE in the llvm.compiler.used set, or is that not a correct interpretation? Going back to the email discussion I included in my earlier response, it seems that it must be true that "The frontend is responsible to add [local ref] @b to llvm.compiler_used", in which case the handling in this current patch that looks for locals in the llvm.compiler.used is correct, right?

To be more precise: the FE can put uses from inline ASM to static symbol in either llvm.used or llvm.compiler_used (i.e. the latter should be "enough").

Only module-level asm, not "inline asm calls", which are opaque to the compiler and require a user attribute like:

(for clang, this is done by the user who usually in C++ uses __attribute__((used)), which translates into llvm.used)

Let me know if I have addressed your concerns with my other responses below.

lib/Analysis/ModuleSummaryAnalysis.cpp

208

I thought I did answer that: "Uses in inline asm must be marked in the source with “attribute((used))”, which causes them to be included in the llvm.used set." As you noted also, this causes those to go into the llvm.used (not llvm.compiler.used). I will make an explicit note here about these not being in the llvm.compiler.used set, and therefore not needing to affect functions that make inline asm calls.

252

Because as I noted in the comment added around the earlier handling, only the llvm.used set locals may be referenced by functions that make inline asm calls, and must be marked explicitly by the user via attribute((used)), which end up in llvm.used only. We only need to collect the llvm.compiler.used here to mark those as NoRename.

test/ThinLTO/X86/module_asm2.ll

74

Will do.

[mehdi_amini]

In D26146#589387, @tejohnson wrote:

In D26146#589364, @mehdi_amini wrote:

In D26146#589341, @tejohnson wrote:

In D26146#585633, @mehdi_amini wrote:

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?

Right for llvm.used, but I was referring to static (local linkage) values in the llvm.compiler.used which is added in this patch, since you mentioned: "It does not apply to non-static I believe, and this is why are currently parsing the module inline ASM at LTO time and updating the llvm.compiler_used, because this is not something the FE is required to do."

I read from that that any statics would have been placed by the FE in the llvm.compiler.used set, or is that not a correct interpretation? Going back to the email discussion I included in my earlier response, it seems that it must be true that "The frontend is responsible to add [local ref] @b to llvm.compiler_used", in which case the handling in this current patch that looks for locals in the llvm.compiler.used is correct, right?

To be more precise: the FE can put uses from inline ASM to static symbol in either llvm.used or llvm.compiler_used (i.e. the latter should be "enough").

Only module-level asm, not "inline asm calls", which are opaque to the compiler and require a user attribute like:

This is not clear to my why?
It seems perfectly legit to me that a FE would generate inline ASM calls to a local function and add it to llvm.compiler_used and not llvm.used.

[tejohnson]

lib/Analysis/ModuleSummaryAnalysis.cpp

252

only the llvm.used set locals may be referenced by functions that make inline asm calls

That should be "only the llvm.used set locals may be referenced from within inline asm calls, and need to be handled by the inline-asm-calling functions above."

[tejohnson]

Address more review comments

[tejohnson]

In D26146#589393, @mehdi_amini wrote:

It seems perfectly legit to me that a FE would generate inline ASM calls to a local function and add it to llvm.compiler_used and not llvm.used.

As discussed on IRC, I now have the same handling for inline asm and module level asm.

[mehdi_amini]

LGTM, thanks.
(Ideally we should have a test that verifies that the encoding in the bitcode itself is as expected with llvm-bcanalyzer, but such tests are more difficult to maintain).

lib/Analysis/ModuleSummaryAnalysis.cpp

286

Use isa<> instead of dyn_cast<> if you don't use the result.

[mehdi_amini]

(See also one nit inline)

[tejohnson]

In D26146#589843, @mehdi_amini wrote:

(See also one nit inline)

Got it. Thanks!

lib/Analysis/ModuleSummaryAnalysis.cpp

286

Fixed.

include/llvm/IR/ModuleSummaryIndex.h

auto Summary =
                         return Summary->modulePath() == ModuleId;
                       });
     if (Summary == CalleeInfoList->second.end())
       return nullptr;
     return Summary->get();
   }
 
   /// Returns the first GlobalValueSummary for \p GV, asserting that there
-  /// is only one if \p PerModuleIndex.
+  /// is only one if \p PerModuleIndex. If \p AssertExists is true, then
+  /// the entry is expected to exist in the index, otherwise, return null
+  /// if not found.
   GlobalValueSummary *getGlobalValueSummary(const GlobalValue &GV,
-                                            bool PerModuleIndex = true) const {
+                                            bool PerModuleIndex = true,
+                                            bool AssertExists = true) const {
     assert(GV.hasName() && "Can't get GlobalValueSummary for GV with no name");
     return getGlobalValueSummary(GlobalValue::getGUID(GV.getName()),
-                                 PerModuleIndex);
+                                 PerModuleIndex, AssertExists);
   }
 
   /// Returns the first GlobalValueSummary for \p ValueGUID, asserting that
-  /// there
-  /// is only one if \p PerModuleIndex.
+  /// there is only one if \p PerModuleIndex. If \p AssertExists is true, then
+  /// the entry is expected to exist in the index, otherwise, return null
+  /// if not found.
   GlobalValueSummary *getGlobalValueSummary(GlobalValue::GUID ValueGUID,
-                                            bool PerModuleIndex = true) const;
+                                            bool PerModuleIndex = true,
+                                            bool AssertExists = true) const;
 
   /// Table of modules, containing module hash and id.
   const StringMap<std::pair<uint64_t, ModuleHash>> &modulePaths() const {
     return ModulePathStringTable;
   }
 
   /// Table of modules, containing hash and id.
   StringMap<std::pair<uint64_t, ModuleHash>> &modulePaths() {

include/llvm/Support/TargetRegistry.h

public:
   bool hasJIT() const { return HasJIT; }
 
   /// hasTargetMachine - Check if this target supports code generation.
   bool hasTargetMachine() const { return TargetMachineCtorFn != nullptr; }
 
   /// hasMCAsmBackend - Check if this target supports .o generation.
   bool hasMCAsmBackend() const { return MCAsmBackendCtorFn != nullptr; }
 
+  /// hasMCAsmParser - Check if this target supports assembly parsing.
+  bool hasMCAsmParser() const { return MCAsmParserCtorFn != nullptr; }
+
   /// @}
   /// @name Feature Constructors
   /// @{
 
   /// createMCAsmInfo - Create a MCAsmInfo implementation for the specified
   /// target triple.
   ///
   /// \param TheTriple This argument is used to determine the target machine

lib/Analysis/LLVMBuild.txt

 ;   http://llvm.org/docs/LLVMBuild.html
 ;
 ;===------------------------------------------------------------------------===;
 
 [component_0]
 type = Library
 name = Analysis
 parent = Libraries
-required_libraries = Core Support ProfileData
+required_libraries = Core Support ProfileData Object

lib/Analysis/ModuleSummaryAnalysis.cpp

 //===- ModuleSummaryAnalysis.cpp - Module summary index builder -----------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This pass builds a ModuleSummaryIndex object for the module, to be written
 // to bitcode or LLVM assembly.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Analysis/ModuleSummaryAnalysis.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/BlockFrequencyInfo.h"
 #include "llvm/Analysis/BlockFrequencyInfoImpl.h"
 #include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/IndirectCallPromotionAnalysis.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/InstIterator.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/ValueSymbolTable.h"
+#include "llvm/Object/IRObjectFile.h"
 #include "llvm/Pass.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "module-summary-analysis"
 
 // Walk through the operands of a given User via worklist iteration and populate
 // the set of GlobalValue references encountered. Invoked either on an
 // Instruction or a GlobalVariable (which walks its initializer).
ModuleSummaryIndex llvm::buildModuleSummaryIndex(
   // exported as they would then require renaming and promotion, but we
   // may have opaque uses e.g. in inline asm.
   SmallPtrSet<GlobalValue *, 8> Used;
   collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false);
   SmallPtrSet<GlobalValue *, 8> LocalsUsed;
   for (auto *V : Used) {
     if (V->hasLocalLinkage())
       LocalsUsed.insert(V);
   }
 
   // Compute summaries for all functions defined in module, and save in the
   // index.
   for (auto &F : M) {
     if (F.isDeclaration())
       continue;
 
     BlockFrequencyInfo *BFI = nullptr;
for (const GlobalVariable &G : M.globals()) {
     computeVariableSummary(Index, G);
   }
 
   // Compute summaries for all aliases defined in module, and save in the
   // index.
   for (const GlobalAlias &A : M.aliases())
     computeAliasSummary(Index, A);
 
+  // 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
+  // they have opaque uses (e.g. in module level asm synthesized by the FE).
+  Used.clear();
+  collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ true);
+  for (auto *V : Used) {
+    if (V->hasLocalLinkage())
+      LocalsUsed.insert(V);
+  }
+
   for (auto *V : LocalsUsed) {
     auto *Summary = Index.getGlobalValueSummary(*V);
     assert(Summary && "Missing summary for global value");
     Summary->setNoRename();
   }
 
+  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.
+    // FIXME: Rename CollectAsmUndefinedRefs to something more general, as we
+    // are also using it to find the file-scope locals defined in module asm.
+    object::IRObjectFile::CollectAsmUndefinedRefs(
+        Triple(M.getTargetTriple()), M.getModuleInlineAsm(),
+        [&M, &Index](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))) {
+            GlobalValue *GV = M.getNamedValue(Name);
+            if (!GV)
+              return;
+            assert(GV->isDeclaration() && "Def in module asm already has definition");
+            GlobalValueSummary::GVFlags Flags(GlobalValue::InternalLinkage,
+                                              /* NoRename */ true,
+                                              /*IsNotViableToInline */ true);
+            // Create the appropriate summary type.
+            if (dyn_cast<Function>(GV)) {
+              std::unique_ptr<FunctionSummary> Summary =
+                  llvm::make_unique<FunctionSummary>(Flags, 0);
+              Summary->setNoRename();
+              Index.addGlobalValueSummary(Name, std::move(Summary));
+            } else {
+              std::unique_ptr<GlobalVarSummary> Summary =
+                  llvm::make_unique<GlobalVarSummary>(Flags);
+              Summary->setNoRename();
+              Index.addGlobalValueSummary(Name, std::move(Summary));
+            }
+          }
+        });
+  }
+
   return Index;
 }
 
 char ModuleSummaryIndexAnalysis::PassID;
 
 ModuleSummaryIndex
 ModuleSummaryIndexAnalysis::run(Module &M, ModuleAnalysisManager &AM) {
   ProfileSummaryInfo &PSI = AM.getResult<ProfileSummaryAnalysis>(M);

lib/Bitcode/Writer/BitcodeWriter.cpp

void ModuleBitcodeWriter::writePerModuleFunctionSummaryRecord(
   NameVals.clear();
 }
 
 // Collect the global value references in the given variable's initializer,
 // and emit them in a summary record.
 void ModuleBitcodeWriter::writeModuleLevelReferences(
     const GlobalVariable &V, SmallVector<uint64_t, 64> &NameVals,
     unsigned FSModRefsAbbrev) {
-  // Only interested in recording variable defs in the summary.
-  if (V.isDeclaration())
+  auto *Summary = Index->getGlobalValueSummary(V, /* PerModuleIndex */ true,
+                                               /* AssertExists */ false);
+  if (!Summary) {
+    // Only declarations should not have a summary (a declaration might however
+    // have a summary if the def was in module level asm).
+    assert(V.isDeclaration());
     return;
+  }
   NameVals.push_back(VE.getValueID(&V));
-  auto *Summary = Index->getGlobalValueSummary(V);
   GlobalVarSummary *VS = cast<GlobalVarSummary>(Summary);
   NameVals.push_back(getEncodedGVSummaryFlags(VS->flags()));
 
   unsigned SizeBeforeRefs = NameVals.size();
   for (auto &RI : VS->refs())
     NameVals.push_back(VE.getValueID(RI.getValue()));
   // Sort the refs for determinism output, the vector returned by FS->refs() has
   // been initialized from a DenseSet.
void ModuleBitcodeWriter::writePerModuleGlobalValueSummary() {
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // flags
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // valueid
   unsigned FSAliasAbbrev = Stream.EmitAbbrev(Abbv);
 
   SmallVector<uint64_t, 64> NameVals;
   // Iterate over the list of functions instead of the Index to
   // ensure the ordering is stable.
   for (const Function &F : M) {
-    if (F.isDeclaration())
-      continue;
     // Summary emission does not support anonymous functions, they have to
     // renamed using the anonymous function renaming pass.
     if (!F.hasName())
       report_fatal_error("Unexpected anonymous function when writing summary");
 
-    auto *Summary = Index->getGlobalValueSummary(F);
+    auto *Summary = Index->getGlobalValueSummary(F, /* PerModuleIndex */ true,
+                                                 /* AssertExists */ false);
+    if (!Summary) {
+      // Only declarations should not have a summary (a declaration might
+      // however have a summary if the def was in module level asm).
+      assert(F.isDeclaration());
+      continue;
+    }
     writePerModuleFunctionSummaryRecord(NameVals, Summary, VE.getValueID(&F),
                                         FSCallsAbbrev, FSCallsProfileAbbrev, F);
   }
 
   // Capture references from GlobalVariable initializers, which are outside
   // of a function scope.
   for (const GlobalVariable &G : M.globals())
     writeModuleLevelReferences(G, NameVals, FSModRefsAbbrev);

lib/IR/ModuleSummaryIndex.cpp

void ModuleSummaryIndex::collectDefinedGVSummariesPerModule(
   for (auto &GlobalList : *this) {
     auto GUID = GlobalList.first;
     for (auto &Summary : GlobalList.second) {
       ModuleToDefinedGVSummaries[Summary->modulePath()][GUID] = Summary.get();
     }
   }
 }
 
-GlobalValueSummary *
-ModuleSummaryIndex::getGlobalValueSummary(uint64_t ValueGUID,
-                                          bool PerModuleIndex) const {
+GlobalValueSummary *ModuleSummaryIndex::getGlobalValueSummary(
+    uint64_t ValueGUID, bool PerModuleIndex, bool AssertExists) const {
   auto SummaryList = findGlobalValueSummaryList(ValueGUID);
+  if (!AssertExists && SummaryList == end())
+    return nullptr;
   assert(SummaryList != end() && "GlobalValue not found in index");
   assert((!PerModuleIndex || SummaryList->second.size() == 1) &&
          "Expected a single entry per global value in per-module index");
   auto &Summary = SummaryList->second[0];
   return Summary.get();
 }

lib/Object/IRObjectFile.cpp

 void IRObjectFile::CollectAsmUndefinedRefs(
     const Triple &TT, StringRef InlineAsm,
     function_ref<void(StringRef, BasicSymbolRef::Flags)> AsmUndefinedRefs) {
   if (InlineAsm.empty())
     return;
 
   std::string Err;
   const Target *T = TargetRegistry::lookupTarget(TT.str(), Err);
-  if (!T)
-    return;
+  assert(T && T->hasMCAsmParser());
 
   std::unique_ptr<MCRegisterInfo> MRI(T->createMCRegInfo(TT.str()));
   if (!MRI)
     return;
 
   std::unique_ptr<MCAsmInfo> MAI(T->createMCAsmInfo(*MRI, TT.str()));
   if (!MAI)
     return;

test/LTO/X86/current-section.ll

 ; RUN: llvm-as < %s >%t1
 ; RUN: llvm-lto -o %t2 %t1
 ; REQUIRES: default_triple
 
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
 module asm ".align 4"

test/ThinLTO/X86/Inputs/module_asm2.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @main({ i64, { i64, i8* }* } %unnamed) #0 {
+  %1 = call i32 @func1() #1
+  %2 = call i32 @func2() #1
+  %3 = call i32 @func3() #1
+  ret i32 %1
+}
+declare i32 @func1() #1
+declare i32 @func2() #1
+declare i32 @func3() #1

test/ThinLTO/X86/module_asm2.ll

+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/module_asm2.ll -o %t2.bc
+
+; RUN: llvm-lto -thinlto-action=run -exported-symbol=main -exported-symbol=func1 -exported-symbol=func2 -exported-symbol=func3 %t1.bc %t2.bc
+; RUN: llvm-nm %t1.bc.thinlto.o | FileCheck  %s --check-prefix=NM0
+; RUN: llvm-nm %t2.bc.thinlto.o | FileCheck  %s --check-prefix=NM1
+
+; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN:     -r=%t1.bc,foo,plx \
+; RUN:     -r=%t1.bc,b,pl \
+; RUN:     -r=%t1.bc,x,pl \
+; RUN:     -r=%t1.bc,func1,pl \
+; RUN:     -r=%t1.bc,func2,pl \
+; RUN:     -r=%t1.bc,func3,pl \
+; RUN:     -r=%t2.bc,main,plx \
+; RUN:     -r=%t2.bc,func1,l \
+; RUN:     -r=%t2.bc,func2,l \
+; RUN:     -r=%t2.bc,func3,l
+; RUN: llvm-nm %t.o.0 | FileCheck  %s --check-prefix=NM0
+; RUN: llvm-nm %t.o.1 | FileCheck  %s --check-prefix=NM1
+
+; NM0-DAG: d b
+; NM0-DAG: d x
+; NM0-DAG: t foo
+; NM0-DAG: T func1
+; NM0-DAG: T func2
+; NM0-DAG: T func3
+
+; NM1-NOT: foo
+; NM1-NOT: b
+; NM1-NOT: x
+; NM1-DAG: U func1
+; NM1-DAG: U func2
+; NM1-DAG: U func3
+; NM1-DAG: T main
+; NM1-NOT: foo
+; NM1-NOT: b
+; NM1-NOT: x
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@b = internal global i32 1, align 4
+@x = internal global i32 1, align 4
+
[e-mail] = appending global [1 x i8*] [i8* bitcast (i32* @b to i8*)], section "llvm.metadata"
[e-mail] = appending global [1 x i8*] [i8* bitcast (i32* @x to i8*)], section "llvm.metadata"
+
+module asm "\09.text"
+module asm "\09.type\09foo,@function"
+module asm "foo:"
+module asm "\09movl    b, %eax"
+module asm "\09movl    x, %edx"
+module asm "\09ret "
+module asm "\09.size\09foo, .-foo"
+module asm ""
+
+declare i16 @foo() #0
+
+define i32 @func1() #1 {
+  call i16 @foo()
+  ret i32 1
+}
+
+define i32 @func2() #1 {
+  %1 = load i32, i32* @b, align 4
+  ret i32 %1
+}
+
+define i32 @func3() #1 {
+  %1 = load i32, i32* @x, align 4
+  ret i32 %1
+}

tools/opt/opt.cpp

int main(int argc, char **argv) {
   EnableDebugBuffering = true;
 
   llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
   LLVMContext Context;
 
   InitializeAllTargets();
   InitializeAllTargetMCs();
   InitializeAllAsmPrinters();
+  InitializeAllAsmParsers();
 
   // Initialize passes
   PassRegistry &Registry = *PassRegistry::getPassRegistry();
   initializeCore(Registry);
   initializeCoroutines(Registry);
   initializeScalarOpts(Registry);
   initializeObjCARCOpts(Registry);
   initializeVectorization(Registry);