[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7841e21c9849: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map… (authored by rahmanl). Repository: rG LLVM Github

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl updated this revision to Diff 291607. rahmanl added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85408/new/ https://reviews.llvm.org/D85408 Files: clang/docs/UsersManual.rst

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl updated this revision to Diff 291595. rahmanl added a comment. - Merge branch 'master' into arcpatch-D85408 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85408/new/ https://reviews.llvm.org/D85408 Files: clang/docs/UsersManual.rst

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl updated this revision to Diff 291589. rahmanl added a comment. - Remove the "labels" part of the clang test as the functionality is tested on LLVM tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85408/new/

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

MaskRay added inline comments. Comment at: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll:1 +; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s + You can drop

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl added a comment. In D85408#2266559 , @dblaikie wrote: > In D85408#2262134 , @rahmanl wrote: > >> @efriedma Would you please chime in specially with respect to @MaskRay 's >> comment about the clang test

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl updated this revision to Diff 291156. rahmanl added a comment. - Remove the complex assembly from the clang test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85408/new/ https://reviews.llvm.org/D85408 Files: clang/docs/UsersManual.rst

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

dblaikie added a comment. In D85408#2262134 , @rahmanl wrote: > @efriedma Would you please chime in specially with respect to @MaskRay 's > comment about the clang test involving assembly? I'll chime in, perhaps @efriedma will/can too: This patch has no

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl added a comment. @efriedma Would you please chime in specially with respect to @MaskRay 's comment about the clang test involving assembly? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85408/new/ https://reviews.llvm.org/D85408

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. I like the simplification:) However, I still feel puzzled by the large chunk of assembly test in `clang/test/CodeGen/basic-block-sections.c`. I hope another reviewer or the code owner can

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl added a comment. Thanks for the review @MaskRay. Is this ready to land now? Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047 + getObjFileLowering().getBBAddrMapSection(*MF.getSection()); + if (!BBAddrMapSection) +return; MaskRay

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl updated this revision to Diff 290511. rahmanl marked an inline comment as done. rahmanl added a comment. - Merge remote-tracking branch 'origin/master' into arcpatch-D85408 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85408/new/

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

MaskRay added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047 + getObjFileLowering().getBBAddrMapSection(*MF.getSection()); + if (!BBAddrMapSection) +return; rahmanl wrote: > MaskRay wrote: > > BBAddrMapSection is always

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047 + getObjFileLowering().getBBAddrMapSection(*MF.getSection()); + if (!BBAddrMapSection) +return; MaskRay wrote: > BBAddrMapSection is always non-null. Delete the

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl updated this revision to Diff 289765. rahmanl marked 5 inline comments as done. rahmanl added a comment. - Address +MaskRay's comments: - Change the check prefix simply to "CHECK" for basic-block-sections-labels.ll - Change the triple to x86_64 for this test. - nits. - Remove the "-LABEL"

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

MaskRay added a comment. Thanks, I have followed the flow. The logic looks good. But I have got some nits and a question about the `clang/test/` assembly test. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1033 +/// * 2nd bit: set if it is a block ending with a tail

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

MaskRay added a comment. In D85408#2253073 , @tmsriram wrote: > In D85408#2253055 , @MaskRay wrote: > >> I am still reading the patch, but I have noticed one thing worth discussing. >> `.bb_addr_map` is a

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

shenhan added a comment. In D85408#2253055 , @MaskRay wrote: > I am still reading the patch, but I have noticed one thing worth discussing. > `.bb_addr_map` is a non-SHF_ALLOC section (meaning that it is not part of the > memory image). An absolute

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

tmsriram added a comment. In D85408#2253055 , @MaskRay wrote: > I am still reading the patch, but I have noticed one thing worth discussing. > `.bb_addr_map` is a non-SHF_ALLOC section (meaning that it is not part of the > memory image). An absolute

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. I am still reading the patch, but I have noticed one thing worth discussing. `.bb_addr_map` is a non-SHF_ALLOC section (meaning that it is not part of the memory image). An

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

MaskRay added a subscriber: rsmith. MaskRay added inline comments. Comment at: clang/test/CodeGen/basic-block-sections.c:31 // BB_LABELS: world: -// BB_LABELS: a.BB.world: -// BB_LABELS: aa.BB.world: -// BB_LABELS: a.BB.another: +// BB_LABELS: .Lfunc_begin0: +// BB_LABELS:

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl added a comment. Thanks a lot for the comments @MaskRay. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85408/new/ https://reviews.llvm.org/D85408 ___ cfe-commits mailing list

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl updated this revision to Diff 289498. rahmanl marked 5 inline comments as done. rahmanl added a comment. - Address @MaskRay's comments. - Rebase with upstream. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85408/new/

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

MaskRay added inline comments. Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:78 } else { + auto Prefix = Ctx.getAsmInfo()->getPrivateLabelPrefix(); CachedMCSymbol = Ctx.getOrCreateSymbol(Twine(Prefix) + "BB" + Expand `auto`

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

MaskRay added inline comments. Comment at: clang/docs/UsersManual.rst:1703 - Controls whether Clang emits a label for each basic block. Further, with - values "all" and "list=arg", each basic block or a subset of basic blocks - can be placed in its own unique section. +

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

snehasish accepted this revision. snehasish added a comment. This revision is now accepted and ready to land. LGTM. Please wait a bit for additional comments from others. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85408/new/

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl marked an inline comment as done. rahmanl added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1030 +/// recursive return edges vs. indirect branches. The format of the metadata is +/// described as follows: 1st bit (LSB): set if this is a return

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl updated this revision to Diff 288694. rahmanl added a comment. - Move getBBAddrMetadata to AsmPrinter as a static function + nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85408/new/ https://reviews.llvm.org/D85408 Files:

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

snehasish added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1030 +/// recursive return edges vs. indirect branches. The format of the metadata is +/// described as follows: 1st bit (LSB): set if this is a return block (return +/// or tail call). 2nd

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl added inline comments. Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:58 +unsigned MachineBasicBlock::getBBAddrMapMetadata() const { + const TargetInstrInfo *TII = getParent()->getSubtarget().getInstrInfo(); snehasish wrote: > I think this method

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl updated this revision to Diff 288158. rahmanl marked 4 inline comments as done. rahmanl added a comment. Address Snehasish's comments. - Move getBBAddrMetadata to AsmPrinter as a static function in AsmPrinter - Remove unnecessary IR instructions in basic-block-sections-labels.ll

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

snehasish added inline comments. Comment at: clang/docs/UsersManual.rst:1706 + its own unique section. With the "labels" value, normal sections are emitted, + but a ".bb_addr_map" section is emitted which would include address offsets + for each basic block in the program,

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl updated this revision to Diff 288105. rahmanl added a comment. Fix failing clang test which used to check for the old ".bb_info" name. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85408/new/ https://reviews.llvm.org/D85408 Files:

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

rahmanl updated this revision to Diff 288104. rahmanl edited the summary of this revision. rahmanl added a comment. Fix failing clang test which used to check for the old ".bb_info" name. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85408/new/