[PATCH] D104060: Machine IR Profile

[Ellis Hoag via Phabricator via llvm-commits]

ellis added a comment.

In D104060#2827459 <https://reviews.llvm.org/D104060#2827459>, @MaskRay wrote:

> @ellis
>
>> The main challenge is storing the offset to the profile data without using dynamic relocations. This is complicated by the fact that we use comdat sections within the __llvm_mipraw section and that ELF does not seem to directly support section relative addresses. The solution is to use PC relative relocations. __start___llvm_mipraw-.Lref gives us the PC relative offset to the start of the raw section and _Z3foov$RAW-.Lref gives us the offset to the profile data for this function relative to the same PC. After we extract the map section, we can subtract these to get the value we want, the section relative raw profile data offset.
>>
>> (_Z3foov$RAW-.Lref) - (__start___llvm_mipraw-.Lref) = _Z3foov$RAW - __start___llvm_mipraw
>
> I am unclear about this. Why does the `$MAP` section needs to know its relative position?

The map section doesn't care about its relative position, but it is used to compute the value we want. Ideally we would do something simple like this to get the section relative address of the symbol.

  _Z3foov$RAW-__start___llvm_mipraw

>From my testing this doesn't work because these symbols are in different section in ELF (because we use comdat sections for the header). Also, IIRC there were other issues due to relocations getting resolved before the final executable was created and ending up with the wrong values. The solution in this patch was the only one that seems to work in all cases.

> Note: if the `$RAW` symbol has a local linkage or has the hidden visibility, a label difference can indeed avoid a dynamic relative relocation.
>
>   # ELF: R_X86_64_PC64
>   .quad .L__profc_foo-.L__profd_foo
>   
>   # Mach-O: a pair of X86_64_RELOC_UNSIGNED and X86_64_RELOC_SUBTRACTOR
>   .quad l___profc_foo-l___profd_foo
>
> Unfortunately this may not be representable in COFF:
>
>   % clang -fprofile-generate a.c -c -target x86_64-windows -o d.o
>   error: Cannot represent this expression

I haven't tested COFF, but i think it might support section relative addresses which would make this format much simpler.

>> We can use the same trick to encode the function address, we just need to also add the address of the raw section which can be looked up in the binary. This is useful to lookup debug info and join it with our final profile data.
>
> A `__profd_$name` variable has a similar field referencing the function. It is used by `IPVK_IndirectCallTarget` so that indirect call target profile can be translated to function names.
>
> We can save the dynamic relocation with the following scheme:
>
>   ; Note the sub constexpr
>   @__profd_main = private global { i64, i64, i64*, i64, i8*, i32, [2 x i16] } { i64 -2624081020897602054, i64 742261418966908927, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc_main, i32 0, i32 0), i64 sub (i64 ptrtoint (i32 ()* @alias_main to i64), i64 ptrtoint ({ i64, i64, i64*, i64, i8*, i32, [2 x i16] }* @__profd_main to i64)), i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profc_main), align 8
>   
>   @alias_main = private alias i32 (), i32 ()* @main
>   
>   define i32 @main() #0 {
>     ...
>
> For other fields of `__profd_$name`, other than the generality and mandatory value profiling (even if you don't use it) issues I mentioned in https://reviews.llvm.org/D104060#2818268 ,
> I think the `$MAP` format is the same.

The format is probably the same. I'm interested to see if we can do something similar to replace our encoded function address.

> The `$MAP` format does not implement these things:
>
> - using private linkage as much as possible

I think we can use private linkage in more places if we need to.

> - ELF comdat any/noduplicates (this increases object file sizes but can enable --gc-sections for linked images)
> - function name compression

We don't care about the size of the map section since it is extracted out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104060