[LTO] Discard non-prevailing defined symbols in module-level assembly

[ychen]

Summary

by inline assembly string replacement. The alternative is using a ad-hoc
MCStreamer and AsmParser, but it is more
expensive to do that and the use cases are very rare that makes it not
worthwhile.

The motivation is that after D90108, the use case demonstrated by the
test case begins to fail. I find the error message is useful hence
this patch.

The change in MachineModuleInfo.cpp is to make sure the error code 1 is
returned. Without it, the error message is printed but llvm-lto2 still
returns 0.

[tejohnson]

I don't love the custom pattern matching approach added here - it is hard to follow and seems potentially brittle. If this case is rare, presumably it is rare that we would have a non-empty set of non prevailing asm symbols, so using something heavier weight shouldn't be an issue if that would work and be clearer.

There are no comments right now so it isn't clear to me what this is doing. What is the behavior before and after the patch? Is this patch fixing an issue created by LTO, or attempting to fix bad input?

I don't follow this comment from the description:

I find the error message is useful hence this patch.

What happens for ThinLTO?

llvm/lib/CodeGen/MachineModuleInfo.cpp

173

This seems like a good change but orthogonal to the issue you are trying to solve - please extract into a different patch along with the associated test changes.

llvm/test/LTO/X86/weak-asm.ll

1

Test case needs some comments.

Also, what is the behavior without this patch?

[ychen]

Thanks for the comment!

In D96931#2574711, @tejohnson wrote:

I don't love the custom pattern matching approach added here - it is hard to follow and seems potentially brittle. If this case is rare, presumably it is rare that we would have a non-empty set of non prevailing asm symbols, so using something heavier weight shouldn't be an issue if that would work and be clearer.

Me neither. One benefit though is that only one place in the source code needs inspection if anything goes off the rail and it doesn't *pollute* other parts of the LTO code. The AsmParser & AsmStreamer solution would (1) involve much more change in LTO/ModuleSymbolTable/MCAsmStreamer (need to create derived class from MCAsmStreamer which is not exposed now) (2) there would be inevitable code sharing/refactoring with ModuleSymbolTable. I really don't think it is a good idea even if it does not impact compile time.

After some thought, I think a third approach is possible: emit the list of to-be-discarded symbol names to a module metadata node and discard these symbols during the normal AsmPrinter::emitInlineAsm process. This should have a minimal compile-time impact, easy implementation. A fourth approach would be to demote the error introduced by D90108 to a warning.

Please let me know which approach is preferred.

There are no comments right now so it isn't clear to me what this is doing. What is the behavior before and after the patch? Is this patch fixing an issue created by LTO, or attempting to fix bad input?

Before this patch, the test fails with "foo changed binding to STB_GLOBAL" because, in the combined LTO module, there is conceptually .weak foo; .global foo hence triggers the error diagnose introduced by D90108. This patch discards the .weak foo part of .weak foo; .global foo to avoid the error diagnosis. I think it is a borderline LTO issue in that we don't handle the external symbols in module asm block with existing symbol resolutions. Although most of the time (including the test case in this patch), the behavior happens to be correct. ThinLTO and non-LTO do not suffer this issue because two bindings (.weak and .global) end up in two object files, the error diagnosis does not trigger then.

[tejohnson]

In D96931#2575999, @ychen wrote:

Thanks for the comment!

In D96931#2574711, @tejohnson wrote:

I don't love the custom pattern matching approach added here - it is hard to follow and seems potentially brittle. If this case is rare, presumably it is rare that we would have a non-empty set of non prevailing asm symbols, so using something heavier weight shouldn't be an issue if that would work and be clearer.

Me neither. One benefit though is that only one place in the source code needs inspection if anything goes off the rail and it doesn't *pollute* other parts of the LTO code. The AsmParser & AsmStreamer solution would (1) involve much more change in LTO/ModuleSymbolTable/MCAsmStreamer (need to create derived class from MCAsmStreamer which is not exposed now) (2) there would be inevitable code sharing/refactoring with ModuleSymbolTable. I really don't think it is a good idea even if it does not impact compile time.

After some thought, I think a third approach is possible: emit the list of to-be-discarded symbol names to a module metadata node and discard these symbols during the normal AsmPrinter::emitInlineAsm process. This should have a minimal compile-time impact, easy implementation. A fourth approach would be to demote the error introduced by D90108 to a warning.

Please let me know which approach is preferred.

There are no comments right now so it isn't clear to me what this is doing. What is the behavior before and after the patch? Is this patch fixing an issue created by LTO, or attempting to fix bad input?

Before this patch, the test fails with "foo changed binding to STB_GLOBAL" because, in the combined LTO module, there is conceptually .weak foo; .global foo hence triggers the error diagnose introduced by D90108. This patch discards the .weak foo part of .weak foo; .global foo to avoid the error diagnosis. I think it is a borderline LTO issue in that we don't handle the external symbols in module asm block with existing symbol resolutions. Although most of the time (including the test case in this patch), the behavior happens to be correct. ThinLTO and non-LTO do not suffer this issue because two bindings (.weak and .global) end up in two object files, the error diagnosis does not trigger then.

Thanks for explaining the situation better. Ideally LTO or MC would mimic the behavior of the linker, which I think would actually be to take the strong def regardless of linking order. When the IR is read and the IRSymbolTable created, the asm symbols are parsed and added so the linker would resolve them. However, I don't have a good idea how to use that when merging the module inline asm together.

@pcc any suggestions?

[MaskRay]

Is this still needed after D97449 ?

[ychen]

In D96931#2598956, @MaskRay wrote:

Is this still needed after D97449 ?

Yes. This is a LTO issue.

[ychen]

Hi @pcc , any suggestion to address this issue? Personally, I think regex to drop the non-prevailing def should be easiest. I'm not sure it is a complete solution but should be good for most cases. Thoughts?

[MaskRay]

OK, I think this makes sense and likely fixes https://github.com/ClangBuiltLinux/linux/issues/1269.

A STB_GLOBAL definition can override a STB_WEAK definition. However, if we stream them in the order (.local foo) foo: (foo is not used, see parseAssignmentExpression) .weak foo; foo: ...

The binding will change to STB_WEAK, while we should actually respect the prevailing definition's binding: STB_LOCAL.

If the symbol is used by regular object files, we may emit a .globl and get a changed binding error.

It is unfortunate that we don't very elegant way nuking non-prevailing inline asm definitions.

Please fix the few suggestions.

llvm/lib/LTO/LTO.cpp

733

StringRef

735

llvm::join

745

auto causes an unnecessary std::string copy.

751

This is not needed. Just let MC ignore whitespace.

llvm/test/LTO/X86/weak-asm.ll

6

%t1 + %t2 is sufficient and makes more sense to the linker (there is no duplicate definition error).

You can add another test using %t2 + %t3.

[MaskRay]

llvm/test/LTO/X86/weak-asm.ll

37

Nit: this can be llvm.compiler.used

llvm.used on ELF get SHF_GNU_RETAIN now. We don't need that stricter semantics.

[ychen]

• Address comments

[ychen]

llvm/include/llvm/MC/MCRegisterInfo.h

170 ↗

(On Diff #329152)

This is to avoid creating a real MCRegisterInfo in getTargetAsmSeparator below.

llvm/lib/LTO/LTO.cpp

733

I could not do PrefixPat + P below with this. PrefixPat.str() + P also works but it seems easier just declaring it string?

[ychen]

llvm/lib/LTO/LTO.cpp

746

This should be able to handle more than one discarded symbols. Verified fixing https://github.com/ClangBuiltLinux/linux/issues/1269

[pcc]

This is just solving part of the problem, no? Imagine that you have a weak definition and a global definition in two object files. Then you will have the same problem.

I think that some kind of marker that we can use to mark blocks of asm as conceptually coming from different object files would be best. But as a short term solution it seems best to disable the binding change diagnostic when full LTO is being used.

[ychen]

In D96931#2612521, @pcc wrote:

This is just solving part of the problem, no? Imagine that you have a weak definition and a global definition in two object files. Then you will have the same problem.

I would think the global one is chosen by the linker?

I think that some kind of marker that we can use to mark blocks of asm as conceptually coming from different object files would be best.

Encoded into metadata: the map<inlineasm string offset, vector<nonprevail symbols>> Then in the MC layer, discard the non-prevail symbols.

[pcc]

In D96931#2612552, @ychen wrote:

In D96931#2612521, @pcc wrote:

This is just solving part of the problem, no? Imagine that you have a weak definition and a global definition in two object files. Then you will have the same problem.

I would think the global one is chosen by the linker?

I thought that the problem was with full LTO where the module inline asm blobs are simply concatenated. So if you have one file with

.global foo
foo:
.byte 1

and another with

.weak foo
foo:
.byte 2

then the assembler will see both foo labels and complain about multiple definitions.

I think that some kind of marker that we can use to mark blocks of asm as conceptually coming from different object files would be best.

Encoded into metadata: the map<inlineasm string offset, vector<nonprevail symbols>> Then in the MC layer, discard the non-prevail symbols.

I mean you would be able to say something like

.new_module
.global foo
foo:
.byte 1

.new_module
.weak foo
foo:
.byte 2

and that would be equivalent to

.global foo
foo:
.byte 1
.byte 2

Similarly if the second file did not provide a definition of foo the assembler would ignore the .weak directive.

[ychen]

In D96931#2612602, @pcc wrote:

In D96931#2612552, @ychen wrote:

In D96931#2612521, @pcc wrote:

This is just solving part of the problem, no? Imagine that you have a weak definition and a global definition in two object files. Then you will have the same problem.

I would think the global one is chosen by the linker?

I thought that the problem was with full LTO where the module inline asm blobs are simply concatenated. So if you have one file with

.global foo
foo:
.byte 1

and another with

.weak foo
foo:
.byte 2

then the assembler will see both foo labels and complain about multiple definitions.

Not sure if I understand you correctly but I'm pretty sure this patch could address this particular example. The newly added test has a similar test case.

I think that some kind of marker that we can use to mark blocks of asm as conceptually coming from different object files would be best.

Encoded into metadata: the map<inlineasm string offset, vector<nonprevail symbols>> Then in the MC layer, discard the non-prevail symbols.

I mean you would be able to say something like

.new_module
.global foo
foo:
.byte 1

.new_module
.weak foo
foo:
.byte 2

and that would be equivalent to

.global foo
foo:
.byte 1
.byte 2

Similarly if the second file did not provide a definition of foo the assembler would ignore the .weak directive.

I don't quite get the part that .byte 2 is following .byte 1. If we don't use LTO, there should be no .byte 2 in the final executable right?

[MaskRay]

The regular LTO precodegen bitcode file %to1.0.5.precodegen.bc is

module asm ".weak foo"    ;;;;;;;;; STB_WEAK definition
module asm "\09 .equ foo,bar"

@llvm.compiler.used = appending global [1 x i8*] [i8* bitcast (i32 (i32)* @bar to i8*)], section "llvm.metadata"

; Function Attrs: norecurse nounwind readnone willreturn
define dso_local i32 @foo(i32 %0) local_unnamed_addr #0 {   ;;;;;;;;;;; STB_GLOBAL definition. Conflict!
  %2 = add nsw i32 %0, 2
  ret i32 %2
}

; Function Attrs: norecurse nounwind readnone willreturn
define internal i32 @bar(i32 %0) #0 {
  %2 = add nsw i32 %0, 1
  ret i32 %2
}

The module-level inline asm is emitted before the functions. The call tree is roughly:

AsmPrinter::doInitialization
  emitInlineAsm (AsmPrinter.cpp:319)
    (void)Parser->Run(/*NoInitialTextSection*/ true, (AsmPrinterInlineAsm.cpp:124)
      ...
        MCELFStreamer::emitSymbolAttribute (set binding to STB_WEAK)
AsmPrinter::emitFunctionBody
  AsmPrinter::emitFunctionHeader
    AsmPrinter::emitLinkage
      MCELFStreamer::emitSymbolAttribute (`error: ... changed binding to STB_GLOBAL`)

A more elegant solution is to let the MCELFStreamer ignore non-prevailing attribute changes (.weak foo) and definitions (.equ foo,bar) during emitInlineAsm.

If that is complex to implement, @pcc having this short-term solution may be useful depending on how urgent @nickdesaulniers wants https://github.com/ClangBuiltLinux/linux/issues/1269 to be fixed.

In https://github.com/ClangBuiltLinux/linux/issues/1269#issuecomment-786963047 , @nickdesaulniers mentioned the binding can change from STB_GLOBAL to STB_WEAK, then STB_WEAK to STB_GLOBAL.
I am curious how that can happen:

  LTO     vmlinux.o
__ia32_compat_sys_sysctl: binding: 1
__ia32_compat_sys_sysctl: binding: 2
__ia32_compat_sys_sysctl: binding: 2
<unknown>:0: error: __ia32_compat_sys_sysctl changed binding to STB_GLOBAL
__ia32_compat_sys_sysctl: binding: 1
LLVM ERROR: Error parsing inline asm

PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.

[ychen]

@MaskRay Thanks for the suggestion.

I have an internal build config that needs this fix. I'm happy to take whatever approach that reviewers agree upon. To summarise the proposed solutions:

• (@pcc) Add assembly directive (.newmodule or just .module), handle the linking in the MC layer.
  • Pros: a general solution that maybe useful in the future, no need to encoding information into IR).
  • Cons: new assembly directive? duplicate linker logic but should be fine since it should be complicated for the use cases.
• (@MaskRay and my initial proposed alternative) discard non-prevailing symbols in MC.
  • Pros: no new assembly directive
  • Cons: through either IR or LLVM API, the symbol list and each symbol's position need to be passed to MC. Better than doing MCAsmParse in LTO phase but still feels like a lot of work for a specific problem.

I would take @pcc 's suggestion. @MaskRay thoughts?

[ychen]

Landed D98762 instead.

llvm/lib/CodeGen/MachineModuleInfo.cpp

 }
 
 void MachineModuleInfo::finalize() {
   Personalities.clear();
 
   delete AddrLabelSymbols;
   AddrLabelSymbols = nullptr;
 
+  if (Context.hadError())
+    TheModule->getContext().emitError("MC error");
+
   Context.reset();
   // We don't clear the ExternalContext.
 
   delete ObjFileMMI;
   ObjFileMMI = nullptr;
 }
 
 MachineModuleInfo::MachineModuleInfo(MachineModuleInfo &&MMI)

llvm/lib/LTO/LTO.cpp

 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMRemarkStreamer.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/LTO/LTOBackend.h"
 #include "llvm/LTO/SummaryBasedOptimizations.h"
 #include "llvm/Linker/IRMover.h"
+#include "llvm/MC/MCAsmInfo.h"
+#include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Object/IRObjectFile.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/VCSRevision.h"
 #include "llvm/Support/raw_ostream.h"
handleNonPrevailingComdat(GlobalValue &GV,
   // errors.
   if (!GV.hasLocalLinkage())
     GV.setLinkage(GlobalValue::AvailableExternallyLinkage);
 
   if (auto GO = dyn_cast<GlobalObject>(&GV))
     GO->setComdat(nullptr);
 }
 
+static const char *getTargetAsmSeparator(const Module &M) {
+  std::string Err;
+  const Triple TT(M.getTargetTriple());
+  const Target *T = TargetRegistry::lookupTarget(TT.str(), Err);
+  assert(T && T->hasMCAsmParser());
+
+  MCTargetOptions MCOptions;
+  MCRegisterInfo MRI;
+  std::unique_ptr<MCAsmInfo> MAI(T->createMCAsmInfo(MRI, TT.str(), MCOptions));
+  if (!MAI)
+    return "\\n";
+  return MAI->getSeparatorString();
+}
+
+static void handleModuleInlineAsm(Module &M,
+                                  std::set<StringRef> &NonPrevailingAsmSymbol) {
+  ModuleSymbolTable::CollectAsmSymvers(M, [&](StringRef Name, StringRef Alias) {
+    if (!NonPrevailingAsmSymbol.count(Alias))
+      NonPrevailingAsmSymbol.erase(Name);
+  });
+
+  if (NonPrevailingAsmSymbol.empty())
+    return;
+
+  std::string IA = M.getModuleInlineAsm();
+  std::string Sep = getTargetAsmSeparator(M);
+  std::string PrefixPat = "(^|[[:blank:]]+)";
+  std::string SuffixPat = "($|" + Sep + ")";
+  std::string Symbols = "(" + join(NonPrevailingAsmSymbol, "|") + ")";
+  std::vector<std::string> Pats = {// Remove assignment directive.
+                                   "\\.(set|equ|equiv|eqv)[[:blank:]]+" +
+                                       Symbols + "[[:blank:]]*,[^" + Sep +
+                                       "]+" + SuffixPat,
+                                   // Remove labels.
+                                   Symbols + ":",
+                                   // Remove symbols.
+                                   "\\.(weak|global|globl)[[:blank:]]+" +
+                                       Symbols + "[[:blank:]]*" + SuffixPat};
+  for (auto P : Pats) {
+    Regex RE(PrefixPat + P, Regex::Newline);
+    assert(RE.isValid());
+    IA = RE.sub("", IA);
+  }
+
+  if (std::all_of(IA.begin(), IA.end(), [](auto c) { return std::isspace(c); }))
+    IA = "";
+  M.setModuleInlineAsm(IA);
+}
+
 // Add a regular LTO object to the link.
 // The resulting module needs to be linked into the combined LTO module with
 // linkRegularLTO.
 Expected<LTO::RegularLTOState::AddedModule>
 LTO::addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
                    const SymbolResolution *&ResI,
                    const SymbolResolution *ResE) {
   RegularLTOState::AddedModule Mod;
while (MsymI != MsymE) {
           !(Flags & object::BasicSymbolRef::SF_FormatSpecific))
         return;
       ++MsymI;
     }
   };
   Skip();
 
   std::set<const Comdat *> NonPrevailingComdats;
+  std::set<StringRef> NonPrevailingAsmSymbol;
   for (const InputFile::Symbol &Sym : Syms) {
     assert(ResI != ResE);
     SymbolResolution Res = *ResI++;
 
     assert(MsymI != MsymE);
     ModuleSymbolTable::Symbol Msym = *MsymI++;
     Skip();
 
if (GlobalValue *GV = Msym.dyn_cast<GlobalValue *>()) {
 
       // Set the 'local' flag based on the linker resolution for this symbol.
       if (Res.FinalDefinitionInLinkageUnit) {
         GV->setDSOLocal(true);
         if (GV->hasDLLImportStorageClass())
           GV->setDLLStorageClass(GlobalValue::DLLStorageClassTypes::
                                  DefaultStorageClass);
       }
-    }
+    } else if (auto *AS = Msym.dyn_cast<ModuleSymbolTable::AsmSymbol *>()) {
+      // Collect non-prevailing symbols.
+      if (!Res.Prevailing)
+        NonPrevailingAsmSymbol.insert(AS->first);
+    } else
+      llvm_unreachable("unknown symbol type");
+
     // Common resolution: collect the maximum size/alignment over all commons.
     // We also record if we see an instance of a common as prevailing, so that
     // if none is prevailing we can ignore it later.
     if (Sym.isCommon()) {
       // FIXME: We should figure out what to do about commons defined by asm.
       // For now they aren't reported correctly by ModuleSymbolTable.
       auto &CommonRes = RegularLTO.Commons[std::string(Sym.getIRName())];
       CommonRes.Size = std::max(CommonRes.Size, Sym.getCommonSize());
       MaybeAlign SymAlign(Sym.getCommonAlignment());
       if (SymAlign)
         CommonRes.Align = max(*SymAlign, CommonRes.Align);
       CommonRes.Prevailing |= Res.Prevailing;
     }
-
   }
+
   if (!M.getComdatSymbolTable().empty())
     for (GlobalValue &GV : M.global_values())
       handleNonPrevailingComdat(GV, NonPrevailingComdats);
+
+  if (!NonPrevailingAsmSymbol.empty())
+    handleModuleInlineAsm(M, NonPrevailingAsmSymbol);
+
   assert(MsymI == MsymE);
   return std::move(Mod);
 }
 
 Error LTO::linkRegularLTO(RegularLTOState::AddedModule Mod,
                           bool LivenessFromIndex) {
   std::vector<GlobalValue *> Keep;
   for (GlobalValue *GV : Mod.Keep) {

llvm/test/CodeGen/AMDGPU/lds-initializer.ll

-; RUN: llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
-; RUN: llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
 
 ; CHECK: lds: unsupported initializer for address space
 
 @lds = addrspace(3) global [8 x i32] [i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8]
 
 define amdgpu_kernel void @load_init_lds_global(i32 addrspace(1)* %out, i1 %p) {
  %gep = getelementptr [8 x i32], [8 x i32] addrspace(3)* @lds, i32 0, i32 10
   %ld = load i32, i32 addrspace(3)* %gep
   store i32 %ld, i32 addrspace(1)* %out
   ret void
 }

llvm/test/CodeGen/AMDGPU/lds-zero-initializer.ll

-; RUN: llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
-; RUN: llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tahiti < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc -march=amdgcn -mcpu=tonga < %s -o /dev/null 2>&1 | FileCheck %s
 
 ; CHECK: lds: unsupported initializer for address space
 
 @lds = addrspace(3) global [256 x i32] zeroinitializer
 
 define amdgpu_kernel void @load_zeroinit_lds_global(i32 addrspace(1)* %out, i1 %p) {
  %gep = getelementptr [256 x i32], [256 x i32] addrspace(3)* @lds, i32 0, i32 10
   %ld = load i32, i32 addrspace(3)* %gep
   store i32 %ld, i32 addrspace(1)* %out
   ret void
 }

llvm/test/CodeGen/XCore/section-name.ll

-; RUN: llc < %s -march=xcore -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llc < %s -march=xcore -o /dev/null 2>&1 | FileCheck %s
 
 @bar = internal global i32 zeroinitializer
 
 define void @".dp.bss"() {
   ret void
 }
 
 ; CHECK: <unknown>:0: error: invalid symbol redefinition

llvm/test/LTO/X86/weak-asm.ll

+; RUN: split-file %s %t
+; RUN: opt %t/t1.ll -o %t1
+; RUN: opt %t/t2.ll -o %t2
+; RUN: opt %t/t3.ll -o %t3
+; RUN: llvm-lto2 run -o %t4 -save-temps %t1 %t2 %t3 \
+; RUN:  -r %t1,foo,px \
+; RUN:  -r %t2,foo, \
+; RUN:  -r %t2,bar,pl \
+; RUN:  -r %t3,foo,
+; RUN: llvm-dis < %t4.0.0.preopt.bc -o - | FileCheck %s --check-prefix=ASM
+; RUN: llvm-nm %t4.0 | FileCheck %s --check-prefix=SYM
+; RUN: llvm-objdump -d --disassemble-symbols=foo %t4.0 \
+; RUN:   | FileCheck %s --check-prefix=DEF
+
+; ASM-NOT: module asm
+
+; SYM: T foo
+
+; DEF: leal    2(%rdi), %eax
+
+;--- t1.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define dso_local i32 @foo(i32 %0) {
+  %2 = add nsw i32 %0, 2
+  ret i32 %2
+}
+
+;--- t2.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+module asm ".weak foo"
+module asm ".equ foo, bar"
+
[e-mail] = appending global [1 x i8*] [i8* bitcast (i32 (i32)* @bar to i8*)], section "llvm.metadata"
+
+define internal i32 @bar(i32 %0) {
+  %2 = add nsw i32 %0, 1
+  ret i32 %2
+}
+
+;--- t3.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+module asm " .global foo ; foo: "