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

Wed Sep 2 14:14:18 PDT 2020

MaskRay added a comment.

In D85408#2253073 <https://reviews.llvm.org/D85408#2253073>, @tmsriram wrote:

> In D85408#2253055 <https://reviews.llvm.org/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 relocation type (`.quad	.Lfunc_begin0`) works but the value is a link-time address, not taking account of the load base (PIE/shared object)). (If .bb_addr_map has the SHF_ALLOC flag, linkers will report a text relocation) How do you intend to use `.bb_addr_map` at runtime?
>
> bb_addr_map is intentionally not loaded as it is never meant to be used at run-time. bb_addr_map will be later be used in conjunction with a hardware profile of the binary to associate executed basic block addresses to machine basic blocks.  The hardware profile of the binary has enough info to determine the load base address with PIE, this is pretty straight forward and multiple strategies exist.  What exactly is the concern here?

Thanks for the explanation! I guessed that non-SHF_ALLOC is intended but hoped that it can confirmed (and you did!). This sections works similar to `.stack_sizes` emitted by clang -fstack-size-section.

This is important because for SHF_ALLOC, SHF_WRITE is also needed to avoid text relocations. I am still reading the patch to ensure that it will work as intended.

================
Comment at: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll:21
+; CHECK:	.section .text._Z3foov,"ax", at progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
----------------
Consider align the content. Since `CHECK-LABEL:` is the longest. 

```
; CHECK:        .section .text._Z3foov,"ax", at progbits
; CHECK-LABEL:	_Z3foov:
; CHECK-NEXT:     [[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
```

Non-labels have additional indentation to make it clear they are bodies covered by labels.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85408/new/

https://reviews.llvm.org/D85408