[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.
Rahman Lavaee via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 26 19:10:00 PDT 2020
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 only relies on already public methods of MachineBasicBlock. It would be cleaner to move this from here to a static helper function in AsmPrinter.cpp. This way we don't add yet another method to the MachineBasicBlock class and keeps the relevant metadata generation logic close to where it is used.
>
> Feel free to ignore if you plan on calling this method elsewhere apart from AsmPrinter.cpp but I can't think of a reason to do so.
Thanks for the suggestion. It makes the code much more readable since I just place that function above emitBBAddrMapSection.
I can even make it a lambda function within emitBBAddrMapSection. WDYT?
================
Comment at: llvm/test/CodeGen/X86/basic-block-sections-labels.ll:5-9
%2 = alloca i8, align 1
%3 = zext i1 %0 to i8
store i8 %3, i8* %2, align 1
%4 = load i8, i8* %2, align 1
%5 = trunc i8 %4 to i1
----------------
snehasish wrote:
> I think this can be removed and the branch on L10 can directly use the param like so:
>
> ```
> define void @_Z3bazb(i1 zeroext %0) personality i32 (...)* @__gxx_personality_v0 {
> br i1 %0, label %6, label %11
> ....
> }
> ```
>
> You will need to adjust the respective virtual register names since %2-%5 is no longer used.
Great suggestion. Thanks. This was copied over from other basic-block-sections tests, so maybe they can be updated too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85408/new/
https://reviews.llvm.org/D85408
More information about the llvm-commits
mailing list