From 570033ed62137374819f8ce446eda31b7aa8f58e Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Tue, 12 May 2020 14:07:50 -0700 Subject: [PATCH] Add nomerge function attribute to supress tail merge optimization in simplifyCFG We want to add a way to avoid merging identical calls so as to keep the separate debug-information for those calls. There is also an asan usecase where having this attribute would be beneficial to avoid alternative work-arounds. Here is the link to the feature request: https://bugs.llvm.org/show_bug.cgi?id=42783. `nomerge` is different from `noline`. `noinline` prevents function from inlining at callsites, but `nomerge` prevents multiple identical calls from being merged into one. This patch adds `nomerge` to disable the optimization in IR level. A followup patch will be needed to let backend understands `nomerge` and avoid tail merge at backend. Reviewed By: asbirlea, rnk Differential Revision: https://reviews.llvm.org/D78659 --- docs/LangRef.rst | 8 +++ include/llvm/Bitcode/LLVMBitCodes.h | 1 + include/llvm/IR/Attributes.td | 3 ++ include/llvm/IR/InstrTypes.h | 3 ++ lib/AsmParser/LLLexer.cpp | 1 + lib/AsmParser/LLParser.cpp | 3 ++ lib/AsmParser/LLToken.h | 1 + lib/Bitcode/Reader/BitcodeReader.cpp | 2 + lib/Bitcode/Writer/BitcodeWriter.cpp | 2 + lib/IR/Attributes.cpp | 2 + lib/IR/Verifier.cpp | 1 + lib/Transforms/Utils/CodeExtractor.cpp | 1 + lib/Transforms/Utils/SimplifyCFG.cpp | 11 +++- test/Transforms/SimplifyCFG/nomerge.ll | 71 ++++++++++++++++++++++++++ 14 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 test/Transforms/SimplifyCFG/nomerge.ll diff --git a/docs/LangRef.rst b/docs/LangRef.rst index 001d3ed7ac6..5845c62a291 100644 --- a/docs/LangRef.rst +++ b/docs/LangRef.rst @@ -1528,6 +1528,14 @@ example: This attribute indicates that the inliner should never inline this function in any situation. This attribute may not be used together with the ``alwaysinline`` attribute. +``nomerge`` + This attribute indicates that calls to this function should never be merged + during optimization. For example, it will prevent tail merging otherwise + identical code sequences that raise an exception or terminate the program. + Tail merging normally reduces the precision of source location information, + making stack traces less useful for debugging. This attribute gives the + user control over the tradeoff between code size and debug information + precision. ``nonlazybind`` This attribute suppresses lazy symbol binding for the function. This may make calls to the function faster, at the cost of extra program diff --git a/include/llvm/Bitcode/LLVMBitCodes.h b/include/llvm/Bitcode/LLVMBitCodes.h index 9b2a3da4c89..e614337e585 100644 --- a/include/llvm/Bitcode/LLVMBitCodes.h +++ b/include/llvm/Bitcode/LLVMBitCodes.h @@ -634,6 +634,7 @@ enum AttributeKindCodes { ATTR_KIND_NOSYNC = 63, ATTR_KIND_SANITIZE_MEMTAG = 64, ATTR_KIND_PREALLOCATED = 65, + ATTR_KIND_NO_MERGE = 66, }; enum ComdatSelectionKindCodes { diff --git a/include/llvm/IR/Attributes.td b/include/llvm/IR/Attributes.td index 5095728748f..63b85eb7993 100644 --- a/include/llvm/IR/Attributes.td +++ b/include/llvm/IR/Attributes.td @@ -103,6 +103,9 @@ def NoInline : EnumAttr<"noinline">; /// Function is called early and/or often, so lazy binding isn't worthwhile. def NonLazyBind : EnumAttr<"nonlazybind">; +/// Disable merging for call sites +def NoMerge : EnumAttr<"nomerge">; + /// Pointer is known to be not null. def NonNull : EnumAttr<"nonnull">; diff --git a/include/llvm/IR/InstrTypes.h b/include/llvm/IR/InstrTypes.h index 37e5e42ee8f..fd34bf30b90 100644 --- a/include/llvm/IR/InstrTypes.h +++ b/include/llvm/IR/InstrTypes.h @@ -1715,6 +1715,9 @@ public: addAttribute(AttributeList::FunctionIndex, Attribute::NoDuplicate); } + /// Determine if the call cannot be tail merged. + bool cannotMerge() const { return hasFnAttr(Attribute::NoMerge); } + /// Determine if the invoke is convergent bool isConvergent() const { return hasFnAttr(Attribute::Convergent); } void setConvergent() { diff --git a/lib/AsmParser/LLLexer.cpp b/lib/AsmParser/LLLexer.cpp index b414b5908c8..06631fc0f7b 100644 --- a/lib/AsmParser/LLLexer.cpp +++ b/lib/AsmParser/LLLexer.cpp @@ -658,6 +658,7 @@ lltok::Kind LLLexer::LexIdentifier() { KEYWORD(noinline); KEYWORD(norecurse); KEYWORD(nonlazybind); + KEYWORD(nomerge); KEYWORD(nonnull); KEYWORD(noredzone); KEYWORD(noreturn); diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp index 1948177916e..0d9751939ab 100644 --- a/lib/AsmParser/LLParser.cpp +++ b/lib/AsmParser/LLParser.cpp @@ -1306,6 +1306,7 @@ bool LLParser::ParseFnAttributeValuePairs(AttrBuilder &B, B.addAttribute(Attribute::NoImplicitFloat); break; case lltok::kw_noinline: B.addAttribute(Attribute::NoInline); break; case lltok::kw_nonlazybind: B.addAttribute(Attribute::NonLazyBind); break; + case lltok::kw_nomerge: B.addAttribute(Attribute::NoMerge); break; case lltok::kw_noredzone: B.addAttribute(Attribute::NoRedZone); break; case lltok::kw_noreturn: B.addAttribute(Attribute::NoReturn); break; case lltok::kw_nosync: B.addAttribute(Attribute::NoSync); break; @@ -1698,6 +1699,7 @@ bool LLParser::ParseOptionalParamAttrs(AttrBuilder &B) { case lltok::kw_noimplicitfloat: case lltok::kw_noinline: case lltok::kw_nonlazybind: + case lltok::kw_nomerge: case lltok::kw_noredzone: case lltok::kw_noreturn: case lltok::kw_nocf_check: @@ -1797,6 +1799,7 @@ bool LLParser::ParseOptionalReturnAttrs(AttrBuilder &B) { case lltok::kw_noimplicitfloat: case lltok::kw_noinline: case lltok::kw_nonlazybind: + case lltok::kw_nomerge: case lltok::kw_noredzone: case lltok::kw_noreturn: case lltok::kw_nocf_check: diff --git a/lib/AsmParser/LLToken.h b/lib/AsmParser/LLToken.h index 74e313289eb..ffced33253e 100644 --- a/lib/AsmParser/LLToken.h +++ b/lib/AsmParser/LLToken.h @@ -204,6 +204,7 @@ enum Kind { kw_noinline, kw_norecurse, kw_nonlazybind, + kw_nomerge, kw_nonnull, kw_noredzone, kw_noreturn, diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index f36dab3c9f7..6e9ff6e9f8f 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -1442,6 +1442,8 @@ static Attribute::AttrKind getAttrFromCode(uint64_t Code) { return Attribute::NoInline; case bitc::ATTR_KIND_NO_RECURSE: return Attribute::NoRecurse; + case bitc::ATTR_KIND_NO_MERGE: + return Attribute::NoMerge; case bitc::ATTR_KIND_NON_LAZY_BIND: return Attribute::NonLazyBind; case bitc::ATTR_KIND_NON_NULL: diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp index 0a6741d97f7..3c8e4b2e3ea 100644 --- a/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -647,6 +647,8 @@ static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) { return bitc::ATTR_KIND_NO_INLINE; case Attribute::NoRecurse: return bitc::ATTR_KIND_NO_RECURSE; + case Attribute::NoMerge: + return bitc::ATTR_KIND_NO_MERGE; case Attribute::NonLazyBind: return bitc::ATTR_KIND_NON_LAZY_BIND; case Attribute::NonNull: diff --git a/lib/IR/Attributes.cpp b/lib/IR/Attributes.cpp index 7a8068e6818..ebf5dd9b520 100644 --- a/lib/IR/Attributes.cpp +++ b/lib/IR/Attributes.cpp @@ -375,6 +375,8 @@ std::string Attribute::getAsString(bool InAttrGrp) const { return "noinline"; if (hasAttribute(Attribute::NonLazyBind)) return "nonlazybind"; + if (hasAttribute(Attribute::NoMerge)) + return "nomerge"; if (hasAttribute(Attribute::NonNull)) return "nonnull"; if (hasAttribute(Attribute::NoRedZone)) diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index b1f060737c6..464a052b783 100644 --- a/lib/IR/Verifier.cpp +++ b/lib/IR/Verifier.cpp @@ -1514,6 +1514,7 @@ void Verifier::visitModuleFlagCGProfileEntry(const MDOperand &MDO) { /// Return true if this attribute kind only applies to functions. static bool isFuncOnlyAttr(Attribute::AttrKind Kind) { switch (Kind) { + case Attribute::NoMerge: case Attribute::NoReturn: case Attribute::NoSync: case Attribute::WillReturn: diff --git a/lib/Transforms/Utils/CodeExtractor.cpp b/lib/Transforms/Utils/CodeExtractor.cpp index 02c19223293..e8154ec73ed 100644 --- a/lib/Transforms/Utils/CodeExtractor.cpp +++ b/lib/Transforms/Utils/CodeExtractor.cpp @@ -874,6 +874,7 @@ Function *CodeExtractor::constructFunction(const ValueSet &inputs, case Attribute::NoAlias: case Attribute::NoBuiltin: case Attribute::NoCapture: + case Attribute::NoMerge: case Attribute::NoReturn: case Attribute::NoSync: case Attribute::None: diff --git a/lib/Transforms/Utils/SimplifyCFG.cpp b/lib/Transforms/Utils/SimplifyCFG.cpp index 1fea5415aa8..5bccb7d7b9d 100644 --- a/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/lib/Transforms/Utils/SimplifyCFG.cpp @@ -1300,6 +1300,14 @@ bool SimplifyCFGOpt::HoistThenElseCodeToIf(BranchInst *BI, if (!TTI.isProfitableToHoist(I1) || !TTI.isProfitableToHoist(I2)) return Changed; + // If any of the two call sites has nomerge attribute, stop hoisting. + if (const auto *CB1 = dyn_cast(I1)) + if (CB1->cannotMerge()) + return Changed; + if (const auto *CB2 = dyn_cast(I2)) + if (CB2->cannotMerge()) + return Changed; + if (isa(I1) || isa(I2)) { assert (isa(I1) && isa(I2)); // The debug location is an integral part of a debug info intrinsic @@ -1485,8 +1493,9 @@ static bool canSinkInstructions( // Conservatively return false if I is an inline-asm instruction. Sinking // and merging inline-asm instructions can potentially create arguments // that cannot satisfy the inline-asm constraints. + // If the instruction has nomerge attribute, return false. if (const auto *C = dyn_cast(I)) - if (C->isInlineAsm()) + if (C->isInlineAsm() || C->cannotMerge()) return false; // Each instruction must have zero or one use. diff --git a/test/Transforms/SimplifyCFG/nomerge.ll b/test/Transforms/SimplifyCFG/nomerge.ll new file mode 100644 index 00000000000..c2d58d93b84 --- /dev/null +++ b/test/Transforms/SimplifyCFG/nomerge.ll @@ -0,0 +1,71 @@ +; RUN: opt < %s -O1 -S | FileCheck %s + +; The attribute nomerge prevents the 3 bar() calls from being sunk/hoisted into +; one inside a function. Check that there are still 3 tail calls. + +; Test case for preventing sinking +; CHECK-LABEL: define void @sink +; CHECK: if.then: +; CHECK-NEXT: tail call void @bar() +; CHECK: if.then2: +; CHECK-NEXT: tail call void @bar() +; CHECK: if.end3: +; CHECK-NEXT: tail call void @bar() +define void @sink(i32 %i) { +entry: + switch i32 %i, label %if.end3 [ + i32 5, label %if.then + i32 7, label %if.then2 + ] + +if.then: + tail call void @bar() #0 + br label %if.end3 + +if.then2: + tail call void @bar() #0 + br label %if.end3 + +if.end3: + tail call void @bar() #0 + ret void +} + +; Test case for preventing hoisting +; CHECK-LABEL: define void @hoist +; CHECK: if.then: +; CHECK-NEXT: tail call void @bar() +; CHECK: if.then2: +; CHECK-NEXT: tail call void @bar() +; CHECK: if.end: +; CHECK-NEXT: tail call void @bar() +define void @hoist(i32 %i) { +entry: + %i.addr = alloca i32, align 4 + store i32 %i, i32* %i.addr, align 4 + %0 = load i32, i32* %i.addr, align 4 + %cmp = icmp eq i32 %0, 5 + br i1 %cmp, label %if.then, label %if.else + +if.then: + tail call void @bar() #1 + unreachable + +if.else: + %1 = load i32, i32* %i.addr, align 4 + %cmp1 = icmp eq i32 %i, 7 + br i1 %cmp1, label %if.then2, label %if.end + +if.then2: + tail call void @bar() #1 + unreachable + +if.end: + tail call void @bar() #1 + unreachable +} + +declare void @bar() + +attributes #0 = { nomerge } +attributes #1 = { noreturn nomerge }