[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

rnk added inline comments. Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1305 +if (const auto *CB1 = dyn_cast(I1)) + if (CB1->cannotMerge()) +return Changed; zequanwu wrote: > rnk wrote: > > It seems inconsistent that we don't apply the

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

This revision was automatically updated to reflect the committed changes. Closed by commit rGcb22ab740355: Add nomerge function attribute to supress tail merge optimization in simplifyCFG (authored by zequanwu, committed by rnk). Changed prior to commit:

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu marked an inline comment as done. zequanwu added inline comments. Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1305 +if (const auto *CB1 = dyn_cast(I1)) + if (CB1->cannotMerge()) +return Changed; rnk wrote: > It seems

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu updated this revision to Diff 262977. zequanwu added a comment. add `nomerge` case in `CodeExtractor.cpp` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 Files: llvm/docs/LangRef.rst llvm/include/llvm/Bitcode/LLVMBitCodes.h

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu updated this revision to Diff 262933. zequanwu marked an inline comment as done. zequanwu added a comment. update comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 Files: llvm/docs/LangRef.rst

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

lebedev.ri added inline comments. Comment at: llvm/include/llvm/IR/Attributes.td:106 +/// Disable tail merge for this function +def NoMerge : EnumAttr<"nomerge">; Elsewhere we seem to be preventing any kind of movement, not just disabling inlining of such

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

asbirlea accepted this revision. asbirlea added a comment. This revision is now accepted and ready to land. Thank you for adding this! Please update first sentence in the description to: We want to add a way to avoid merging identical calls so as to keep the separate debug-information for

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

rnk added a comment. I think this looks good, but I would like to get a second opinion from Alina (@asbirlea). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 ___ cfe-commits mailing list

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu updated this revision to Diff 261059. zequanwu marked 4 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 Files: llvm/docs/LangRef.rst llvm/include/llvm/Bitcode/LLVMBitCodes.h

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

rnk added a comment. This looks pretty close, let's do one more iteration. Comment at: llvm/include/llvm/IR/InstrTypes.h:1720 + /// Determine if the invoke cannot be tail merged. + bool cannotMerge() const { return hasFnAttr(Attribute::NoMerge); } Let's

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu updated this revision to Diff 260780. zequanwu added a comment. Add test case for preventing hoisting. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 Files: llvm/docs/LangRef.rst llvm/include/llvm/Bitcode/LLVMBitCodes.h

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu added a comment. > Does sinking and hoisting remove the debug source location? I assumed that it > wouldn't, but now after all the smooth stepping work, maybe it does. Yes, both sinking and hoisting in simplifycfg remove the debug source location by `Instruction::applyMergedLocation`

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu updated this revision to Diff 260529. zequanwu added a comment. Split the the original differential to 2 smaller ones. This one add nomerge function attribute in IR level. Another one will add the attribute to let frontend recognize it. CHANGES SINCE LAST ACTION

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

rnk added a comment. In D78659#2003152 , @zequanwu wrote: > Add check in LICM to prevent sink or hoist on `nomerge` call Does sinking and hoisting remove the debug source location? I assumed that it wouldn't, but now after all the smooth stepping work,

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu updated this revision to Diff 260039. zequanwu added a comment. Add check in LICM to prevent sink or hoist on `nomerge` call CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 Files: clang/include/clang/Basic/Attr.td

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu updated this revision to Diff 260038. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/lib/CodeGen/CGCall.cpp clang/test/CodeGen/attr-nomerge.c

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

asbirlea added a comment. I think that expanding the patch description will help motivate this and clarify the use-cases for which this is useful, and how it differentiates from `noinline`. Perhaps include that the attribute aims to avoid merging identical calls so as to keep the separate

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu updated this revision to Diff 259938. Herald added subscribers: dexonsmith, steven_wu. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu added a comment. In D78659#1997528 , @lebedev.ri wrote: > 1. Tests missing > 2. Why isn't `noinline` sufficient, why a whole new attribue? 1. Test added. 2. We want to avoid merging multiple call sites of same function, but `noinline` cannot

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu updated this revision to Diff 259455. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/lib/CodeGen/CGCall.cpp clang/test/CodeGen/attr-nomerge.c

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

lebedev.ri added a comment. 1. Tests missing 2. Why isn't `noinline` sufficient, why a whole new attribue? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 ___

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

lebedev.ri added a subscriber: llvm-commits. lebedev.ri added a comment. + llvm-commits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 ___ cfe-commits mailing

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

zequanwu created this revision. zequanwu added reviewers: asbirlea, rnk. zequanwu added projects: LLVM, clang. Herald added subscribers: cfe-commits, hiraditya. We want to add a way to avoid tail calls to noreturn function from being merged. This patch add nomerge to disable the optimization in