From 8dc80efd4f54aea90b33b260b658554b596e8d58 Mon Sep 17 00:00:00 2001 From: Yaxun Liu Date: Tue, 30 Jan 2018 22:32:39 +0000 Subject: [PATCH] LLParser: add an argument for overriding data layout and do not check alloca addr space Sometimes users do not specify data layout in LLVM assembly and let llc set the data layout by target triple after loading the LLVM assembly. Currently the parser checks alloca address space no matter whether the LLVM assembly contains data layout definition, which causes false alarm since the default data layout does not contain the correct alloca address space. The parser also calls verifier to check debug info and updating invalid debug info. Currently there is no way to let the verifier to check debug info only. If the verifier finds non-debug-info issues the parser will fail. For llc, the fix is to remove the check of alloca addr space in the parser and disable updating debug info, and defer the updating of debug info and verification to be after setting data layout of the IR by target. For other llvm tools, since they do not override data layout by target but instead can override data layout by a command line option, an argument for overriding data layout is added to the parser. In cases where data layout overriding is necessary for the parser, the data layout can be provided by command line. Differential Revision: https://reviews.llvm.org/D41832 llvm-svn: 323826 --- include/llvm/AsmParser/Parser.h | 16 ++++++--- include/llvm/IRReader/IRReader.h | 9 +++-- lib/AsmParser/LLParser.cpp | 12 ++----- lib/AsmParser/LLParser.h | 11 ++++-- lib/AsmParser/Parser.cpp | 35 ++++++++++--------- lib/CodeGen/MIRParser/MIRParser.cpp | 2 +- lib/IRReader/IRReader.cpp | 13 ++++--- .../datalayout-alloca-addrspace-mismatch-0.ll | 4 ++- .../datalayout-alloca-addrspace-mismatch-1.ll | 4 ++- .../datalayout-alloca-addrspace-mismatch-2.ll | 4 ++- .../drop-debug-info-nonzero-alloca.ll | 25 +++++++++++++ test/CodeGen/AMDGPU/alloca.ll | 12 +++++++ test/CodeGen/AMDGPU/fence-barrier.ll | 35 ++++++++++--------- test/CodeGen/AMDGPU/invalid-alloca.ll | 16 +++++++++ test/CodeGen/AMDGPU/sched-crash-dbg-value.mir | 26 +++++++------- tools/llc/llc.cpp | 23 +++++++----- tools/llvm-as/llvm-as.cpp | 9 +++-- tools/opt/opt.cpp | 12 +++---- 18 files changed, 179 insertions(+), 89 deletions(-) create mode 100644 test/Assembler/drop-debug-info-nonzero-alloca.ll create mode 100644 test/CodeGen/AMDGPU/alloca.ll create mode 100644 test/CodeGen/AMDGPU/invalid-alloca.ll diff --git a/include/llvm/AsmParser/Parser.h b/include/llvm/AsmParser/Parser.h index 5f02e488e5b..4e2210192a0 100644 --- a/include/llvm/AsmParser/Parser.h +++ b/include/llvm/AsmParser/Parser.h @@ -39,9 +39,11 @@ class Type; /// \param UpgradeDebugInfo Run UpgradeDebugInfo, which runs the Verifier. /// This option should only be set to false by llvm-as /// for use inside the LLVM testuite! +/// \param DataLayoutString Override datalayout in the llvm assembly. std::unique_ptr parseAssemblyFile(StringRef Filename, SMDiagnostic &Error, LLVMContext &Context, - SlotMapping *Slots = nullptr, bool UpgradeDebugInfo = true); + SlotMapping *Slots = nullptr, bool UpgradeDebugInfo = true, + StringRef DataLayoutString = ""); /// The function is a secondary interface to the LLVM Assembly Parser. It parses /// an ASCII string that (presumably) contains LLVM Assembly code. It returns a @@ -57,11 +59,13 @@ parseAssemblyFile(StringRef Filename, SMDiagnostic &Error, LLVMContext &Context, /// \param UpgradeDebugInfo Run UpgradeDebugInfo, which runs the Verifier. /// This option should only be set to false by llvm-as /// for use inside the LLVM testuite! +/// \param DataLayoutString Override datalayout in the llvm assembly. std::unique_ptr parseAssemblyString(StringRef AsmString, SMDiagnostic &Error, LLVMContext &Context, SlotMapping *Slots = nullptr, - bool UpgradeDebugInfo = true); + bool UpgradeDebugInfo = true, + StringRef DataLayoutString = ""); /// parseAssemblyFile and parseAssemblyString are wrappers around this function. /// \brief Parse LLVM Assembly from a MemoryBuffer. @@ -72,10 +76,12 @@ std::unique_ptr parseAssemblyString(StringRef AsmString, /// \param UpgradeDebugInfo Run UpgradeDebugInfo, which runs the Verifier. /// This option should only be set to false by llvm-as /// for use inside the LLVM testuite! +/// \param DataLayoutString Override datalayout in the llvm assembly. std::unique_ptr parseAssembly(MemoryBufferRef F, SMDiagnostic &Err, LLVMContext &Context, SlotMapping *Slots = nullptr, - bool UpgradeDebugInfo = true); + bool UpgradeDebugInfo = true, + StringRef DataLayoutString = ""); /// This function is the low-level interface to the LLVM Assembly Parser. /// This is kept as an independent function instead of being inlined into @@ -91,9 +97,11 @@ std::unique_ptr parseAssembly(MemoryBufferRef F, SMDiagnostic &Err, /// \param UpgradeDebugInfo Run UpgradeDebugInfo, which runs the Verifier. /// This option should only be set to false by llvm-as /// for use inside the LLVM testuite! +/// \param DataLayoutString Override datalayout in the llvm assembly. bool parseAssemblyInto(MemoryBufferRef F, Module &M, SMDiagnostic &Err, SlotMapping *Slots = nullptr, - bool UpgradeDebugInfo = true); + bool UpgradeDebugInfo = true, + StringRef DataLayoutString = ""); /// Parse a type and a constant value in the given string. /// diff --git a/include/llvm/IRReader/IRReader.h b/include/llvm/IRReader/IRReader.h index f5621647db0..bedde8954fb 100644 --- a/include/llvm/IRReader/IRReader.h +++ b/include/llvm/IRReader/IRReader.h @@ -15,6 +15,7 @@ #ifndef LLVM_IRREADER_IRREADER_H #define LLVM_IRREADER_IRREADER_H +#include "llvm/ADT/StringRef.h" #include namespace llvm { @@ -40,9 +41,11 @@ getLazyIRFileModule(StringRef Filename, SMDiagnostic &Err, LLVMContext &Context, /// \param UpgradeDebugInfo Run UpgradeDebugInfo, which runs the Verifier. /// This option should only be set to false by llvm-as /// for use inside the LLVM testuite! +/// \param DataLayoutString Override datalayout in the llvm assembly. std::unique_ptr parseIR(MemoryBufferRef Buffer, SMDiagnostic &Err, LLVMContext &Context, - bool UpgradeDebugInfo = true); + bool UpgradeDebugInfo = true, + StringRef DataLayoutString = ""); /// If the given file holds a bitcode image, return a Module for it. /// Otherwise, attempt to parse it as LLVM Assembly and return a Module @@ -50,9 +53,11 @@ std::unique_ptr parseIR(MemoryBufferRef Buffer, SMDiagnostic &Err, /// \param UpgradeDebugInfo Run UpgradeDebugInfo, which runs the Verifier. /// This option should only be set to false by llvm-as /// for use inside the LLVM testuite! +/// \param DataLayoutString Override datalayout in the llvm assembly. std::unique_ptr parseIRFile(StringRef Filename, SMDiagnostic &Err, LLVMContext &Context, - bool UpgradeDebugInfo = true); + bool UpgradeDebugInfo = true, + StringRef DataLayoutString = ""); } #endif diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp index 9bc34f927e8..226e4827968 100644 --- a/lib/AsmParser/LLParser.cpp +++ b/lib/AsmParser/LLParser.cpp @@ -327,7 +327,8 @@ bool LLParser::ParseTargetDefinition() { if (ParseToken(lltok::equal, "expected '=' after target datalayout") || ParseStringConstant(Str)) return true; - M->setDataLayout(Str); + if (DataLayoutStr.empty()) + M->setDataLayout(Str); return false; } } @@ -6261,14 +6262,7 @@ int LLParser::ParseAlloc(Instruction *&Inst, PerFunctionState &PFS) { if (Size && !Size->getType()->isIntegerTy()) return Error(SizeLoc, "element count must have integer type"); - const DataLayout &DL = M->getDataLayout(); - unsigned AS = DL.getAllocaAddrSpace(); - if (AS != AddrSpace) { - // TODO: In the future it should be possible to specify addrspace per-alloca. - return Error(ASLoc, "address space must match datalayout"); - } - - AllocaInst *AI = new AllocaInst(Ty, AS, Size, Alignment); + AllocaInst *AI = new AllocaInst(Ty, AddrSpace, Size, Alignment); AI->setUsedWithInAlloca(IsInAlloca); AI->setSwiftError(IsSwiftError); Inst = AI; diff --git a/lib/AsmParser/LLParser.h b/lib/AsmParser/LLParser.h index 94e4c1ae96d..a3252ff5f19 100644 --- a/lib/AsmParser/LLParser.h +++ b/lib/AsmParser/LLParser.h @@ -143,12 +143,19 @@ namespace llvm { /// UpgradeDebuginfo so it can generate broken bitcode. bool UpgradeDebugInfo; + /// DataLayout string to override that in LLVM assembly. + StringRef DataLayoutStr; + public: LLParser(StringRef F, SourceMgr &SM, SMDiagnostic &Err, Module *M, - SlotMapping *Slots = nullptr, bool UpgradeDebugInfo = true) + SlotMapping *Slots = nullptr, bool UpgradeDebugInfo = true, + StringRef DataLayoutString = "") : Context(M->getContext()), Lex(F, SM, Err, M->getContext()), M(M), Slots(Slots), BlockAddressPFS(nullptr), - UpgradeDebugInfo(UpgradeDebugInfo) {} + UpgradeDebugInfo(UpgradeDebugInfo), DataLayoutStr(DataLayoutString) { + if (!DataLayoutStr.empty()) + M->setDataLayout(DataLayoutStr); + } bool Run(); bool parseStandaloneConstantValue(Constant *&C, const SlotMapping *Slots); diff --git a/lib/AsmParser/Parser.cpp b/lib/AsmParser/Parser.cpp index a43ae2b5577..37b97a17a53 100644 --- a/lib/AsmParser/Parser.cpp +++ b/lib/AsmParser/Parser.cpp @@ -23,31 +23,34 @@ using namespace llvm; bool llvm::parseAssemblyInto(MemoryBufferRef F, Module &M, SMDiagnostic &Err, - SlotMapping *Slots, bool UpgradeDebugInfo) { + SlotMapping *Slots, bool UpgradeDebugInfo, + StringRef DataLayoutString) { SourceMgr SM; std::unique_ptr Buf = MemoryBuffer::getMemBuffer(F); SM.AddNewSourceBuffer(std::move(Buf), SMLoc()); - return LLParser(F.getBuffer(), SM, Err, &M, Slots, UpgradeDebugInfo).Run(); + return LLParser(F.getBuffer(), SM, Err, &M, Slots, UpgradeDebugInfo, + DataLayoutString) + .Run(); } std::unique_ptr llvm::parseAssembly(MemoryBufferRef F, SMDiagnostic &Err, LLVMContext &Context, - SlotMapping *Slots, bool UpgradeDebugInfo) { + SlotMapping *Slots, bool UpgradeDebugInfo, + StringRef DataLayoutString) { std::unique_ptr M = make_unique(F.getBufferIdentifier(), Context); - if (parseAssemblyInto(F, *M, Err, Slots, UpgradeDebugInfo)) + if (parseAssemblyInto(F, *M, Err, Slots, UpgradeDebugInfo, DataLayoutString)) return nullptr; return M; } -std::unique_ptr llvm::parseAssemblyFile(StringRef Filename, - SMDiagnostic &Err, - LLVMContext &Context, - SlotMapping *Slots, - bool UpgradeDebugInfo) { +std::unique_ptr +llvm::parseAssemblyFile(StringRef Filename, SMDiagnostic &Err, + LLVMContext &Context, SlotMapping *Slots, + bool UpgradeDebugInfo, StringRef DataLayoutString) { ErrorOr> FileOrErr = MemoryBuffer::getFileOrSTDIN(Filename); if (std::error_code EC = FileOrErr.getError()) { @@ -57,16 +60,16 @@ std::unique_ptr llvm::parseAssemblyFile(StringRef Filename, } return parseAssembly(FileOrErr.get()->getMemBufferRef(), Err, Context, Slots, - UpgradeDebugInfo); + UpgradeDebugInfo, DataLayoutString); } -std::unique_ptr llvm::parseAssemblyString(StringRef AsmString, - SMDiagnostic &Err, - LLVMContext &Context, - SlotMapping *Slots, - bool UpgradeDebugInfo) { +std::unique_ptr +llvm::parseAssemblyString(StringRef AsmString, SMDiagnostic &Err, + LLVMContext &Context, SlotMapping *Slots, + bool UpgradeDebugInfo, StringRef DataLayoutString) { MemoryBufferRef F(AsmString, ""); - return parseAssembly(F, Err, Context, Slots, UpgradeDebugInfo); + return parseAssembly(F, Err, Context, Slots, UpgradeDebugInfo, + DataLayoutString); } Constant *llvm::parseConstantValue(StringRef Asm, SMDiagnostic &Err, diff --git a/lib/CodeGen/MIRParser/MIRParser.cpp b/lib/CodeGen/MIRParser/MIRParser.cpp index e4e3fbbd75d..52ac08cd2d3 100644 --- a/lib/CodeGen/MIRParser/MIRParser.cpp +++ b/lib/CodeGen/MIRParser/MIRParser.cpp @@ -237,7 +237,7 @@ std::unique_ptr MIRParserImpl::parseIRModule() { dyn_cast_or_null(In.getCurrentNode())) { SMDiagnostic Error; M = parseAssembly(MemoryBufferRef(BSN->getValue(), Filename), Error, - Context, &IRSlots); + Context, &IRSlots, /*UpgradeDebugInfo=*/false); if (!M) { reportDiagnostic(diagFromBlockStringDiag(Error, BSN->getSourceRange())); return nullptr; diff --git a/lib/IRReader/IRReader.cpp b/lib/IRReader/IRReader.cpp index 999f11deb15..36bbf719bb6 100644 --- a/lib/IRReader/IRReader.cpp +++ b/lib/IRReader/IRReader.cpp @@ -68,7 +68,8 @@ std::unique_ptr llvm::getLazyIRFileModule(StringRef Filename, std::unique_ptr llvm::parseIR(MemoryBufferRef Buffer, SMDiagnostic &Err, LLVMContext &Context, - bool UpgradeDebugInfo) { + bool UpgradeDebugInfo, + StringRef DataLayoutString) { NamedRegionTimer T(TimeIRParsingName, TimeIRParsingDescription, TimeIRParsingGroupName, TimeIRParsingGroupDescription, TimePassesIsEnabled); @@ -83,15 +84,19 @@ std::unique_ptr llvm::parseIR(MemoryBufferRef Buffer, SMDiagnostic &Err, }); return nullptr; } + if (!DataLayoutString.empty()) + ModuleOrErr.get()->setDataLayout(DataLayoutString); return std::move(ModuleOrErr.get()); } - return parseAssembly(Buffer, Err, Context, nullptr, UpgradeDebugInfo); + return parseAssembly(Buffer, Err, Context, nullptr, UpgradeDebugInfo, + DataLayoutString); } std::unique_ptr llvm::parseIRFile(StringRef Filename, SMDiagnostic &Err, LLVMContext &Context, - bool UpgradeDebugInfo) { + bool UpgradeDebugInfo, + StringRef DataLayoutString) { ErrorOr> FileOrErr = MemoryBuffer::getFileOrSTDIN(Filename); if (std::error_code EC = FileOrErr.getError()) { @@ -101,7 +106,7 @@ std::unique_ptr llvm::parseIRFile(StringRef Filename, SMDiagnostic &Err, } return parseIR(FileOrErr.get()->getMemBufferRef(), Err, Context, - UpgradeDebugInfo); + UpgradeDebugInfo, DataLayoutString); } //===----------------------------------------------------------------------===// diff --git a/test/Assembler/datalayout-alloca-addrspace-mismatch-0.ll b/test/Assembler/datalayout-alloca-addrspace-mismatch-0.ll index 31920183c65..828aa133c76 100644 --- a/test/Assembler/datalayout-alloca-addrspace-mismatch-0.ll +++ b/test/Assembler/datalayout-alloca-addrspace-mismatch-0.ll @@ -2,7 +2,9 @@ target datalayout = "A1" -; CHECK: :7:41: error: address space must match datalayout +; CHECK: Allocation instruction pointer not in the stack address space! +; CHECK-NEXT: %alloca_scalar_no_align = alloca i32, addrspace(2) + define void @use_alloca() { %alloca_scalar_no_align = alloca i32, addrspace(2) ret void diff --git a/test/Assembler/datalayout-alloca-addrspace-mismatch-1.ll b/test/Assembler/datalayout-alloca-addrspace-mismatch-1.ll index 8778a05291c..9f1b91aae61 100644 --- a/test/Assembler/datalayout-alloca-addrspace-mismatch-1.ll +++ b/test/Assembler/datalayout-alloca-addrspace-mismatch-1.ll @@ -2,7 +2,9 @@ target datalayout = "A1" -; CHECK: :7:50: error: address space must match datalayout +; CHECK: Allocation instruction pointer not in the stack address space! +; CHECK-NEXT: %alloca_scalar_no_align = alloca i32, align 4, addrspace(2) + define void @use_alloca() { %alloca_scalar_no_align = alloca i32, align 4, addrspace(2) ret void diff --git a/test/Assembler/datalayout-alloca-addrspace-mismatch-2.ll b/test/Assembler/datalayout-alloca-addrspace-mismatch-2.ll index b6e2738a4f6..1e9d3cb959c 100644 --- a/test/Assembler/datalayout-alloca-addrspace-mismatch-2.ll +++ b/test/Assembler/datalayout-alloca-addrspace-mismatch-2.ll @@ -2,7 +2,9 @@ target datalayout = "A1" -; CHECK: :7:50: error: address space must match datalayout +; CHECK: Allocation instruction pointer not in the stack address space! +; CHECK-NEXT: %alloca_scalar_no_align = alloca i32, align 4, addrspace(2), !foo !0 + define void @use_alloca() { %alloca_scalar_no_align = alloca i32, align 4, addrspace(2), !foo !0 ret void diff --git a/test/Assembler/drop-debug-info-nonzero-alloca.ll b/test/Assembler/drop-debug-info-nonzero-alloca.ll new file mode 100644 index 00000000000..739edbacde9 --- /dev/null +++ b/test/Assembler/drop-debug-info-nonzero-alloca.ll @@ -0,0 +1,25 @@ +; RUN: llvm-as < %s -o %t.bc -data-layout=A5 2>&1 | FileCheck -check-prefixes=COM,AS %s +; RUN: llvm-dis < %t.bc | FileCheck -check-prefixes=COM,DIS %s +; RUN: opt < %s -S -data-layout=A5 2>&1 | FileCheck -check-prefixes=COM,AS %s +; RUN: opt < %t.bc -S | FileCheck -check-prefixes=COM,DIS %s + +define void @foo() { +entry: +; DIS: target datalayout = "A5" +; DIS: %tmp = alloca i32, addrspace(5) + %tmp = alloca i32, addrspace(5) + call void @llvm.dbg.value( + metadata i8* undef, + metadata !DILocalVariable(scope: !1), + metadata !DIExpression()) +; COM-NOT: Allocation instruction pointer not in the stack address space! +; AS: llvm.dbg.value intrinsic requires a !dbg attachment +; AS: warning: ignoring invalid debug info in +ret void +} + +declare void @llvm.dbg.value(metadata, metadata, metadata) + +!llvm.module.flags = !{!0} +!0 = !{i32 2, !"Debug Info Version", i32 3} +!1 = distinct !DISubprogram(name: "foo") diff --git a/test/CodeGen/AMDGPU/alloca.ll b/test/CodeGen/AMDGPU/alloca.ll new file mode 100644 index 00000000000..fd42e925f3a --- /dev/null +++ b/test/CodeGen/AMDGPU/alloca.ll @@ -0,0 +1,12 @@ +; RUN: llvm-as -data-layout=A5 < %s | llvm-dis | FileCheck %s +; RUN: llc -mtriple amdgcn-amd-amdhsa-amdgiz < %s +; RUN: llvm-as -data-layout=A5 < %s | llc -mtriple amdgcn-amd-amdhsa-amdgiz +; RUN: opt -data-layout=A5 -S < %s +; RUN: llvm-as -data-layout=A5 < %s | opt -S + +; CHECK: %tmp = alloca i32, addrspace(5) +define amdgpu_kernel void @test() { + %tmp = alloca i32, addrspace(5) + ret void +} + diff --git a/test/CodeGen/AMDGPU/fence-barrier.ll b/test/CodeGen/AMDGPU/fence-barrier.ll index f140a762942..8c6c17e5e57 100644 --- a/test/CodeGen/AMDGPU/fence-barrier.ll +++ b/test/CodeGen/AMDGPU/fence-barrier.ll @@ -1,4 +1,5 @@ -; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx803 -enable-si-insert-waitcnts=1 -verify-machineinstrs < %s | FileCheck --check-prefix=GCN %s +; RUN: llc -mtriple=amdgcn-amd-amdhsa-amdgiz -mcpu=gfx803 -enable-si-insert-waitcnts=1 -verify-machineinstrs < %s | FileCheck --check-prefix=GCN %s +; RUN: llvm-as -data-layout=A5 < %s | llc -mtriple=amdgcn-amd-amdhsa-amdgiz -mcpu=gfx803 -enable-si-insert-waitcnts=1 -verify-machineinstrs | FileCheck --check-prefix=GCN %s declare i8 addrspace(2)* @llvm.amdgcn.dispatch.ptr() declare i8 addrspace(2)* @llvm.amdgcn.implicitarg.ptr() @@ -16,8 +17,8 @@ declare void @llvm.amdgcn.s.barrier() ; GCN-NEXT: s_barrier ; GCN: flat_store_dword define amdgpu_kernel void @test_local(i32 addrspace(1)*) { - %2 = alloca i32 addrspace(1)*, align 4 - store i32 addrspace(1)* %0, i32 addrspace(1)** %2, align 4 + %2 = alloca i32 addrspace(1)*, align 4, addrspace(5) + store i32 addrspace(1)* %0, i32 addrspace(1)* addrspace(5)* %2, align 4 %3 = call i32 @llvm.amdgcn.workitem.id.x() %4 = zext i32 %3 to i64 %5 = icmp eq i64 %4, 0 @@ -32,7 +33,7 @@ define amdgpu_kernel void @test_local(i32 addrspace(1)*) { call void @llvm.amdgcn.s.barrier() fence syncscope("workgroup") acquire %8 = load i32, i32 addrspace(3)* getelementptr inbounds ([1 x i32], [1 x i32] addrspace(3)* @test_local.temp, i64 0, i64 0), align 4 - %9 = load i32 addrspace(1)*, i32 addrspace(1)** %2, align 4 + %9 = load i32 addrspace(1)*, i32 addrspace(1)* addrspace(5)* %2, align 4 %10 = call i8 addrspace(2)* @llvm.amdgcn.dispatch.ptr() %11 = call i32 @llvm.amdgcn.workitem.id.x() %12 = call i32 @llvm.amdgcn.workgroup.id.x() @@ -58,14 +59,14 @@ define amdgpu_kernel void @test_local(i32 addrspace(1)*) { ; GCN: s_waitcnt vmcnt(0) lgkmcnt(0){{$}} ; GCN-NEXT: s_barrier define amdgpu_kernel void @test_global(i32 addrspace(1)*) { - %2 = alloca i32 addrspace(1)*, align 4 - %3 = alloca i32, align 4 - store i32 addrspace(1)* %0, i32 addrspace(1)** %2, align 4 - store i32 0, i32* %3, align 4 + %2 = alloca i32 addrspace(1)*, align 4, addrspace(5) + %3 = alloca i32, align 4, addrspace(5) + store i32 addrspace(1)* %0, i32 addrspace(1)* addrspace(5)* %2, align 4 + store i32 0, i32 addrspace(5)* %3, align 4 br label %4 ;