[PATCH] D91878: [CSSPGO] Pseudo probe encoding and emission.

Tue Dec 8 17:27:52 PST 2020

hoy added a comment.

In D91878#2441512 <https://reviews.llvm.org/D91878#2441512>, @wmi wrote:

> In D91878#2441363 <https://reviews.llvm.org/D91878#2441363>, @hoy wrote:
>
>> In D91878#2440875 <https://reviews.llvm.org/D91878#2440875>, @wmi wrote:
>>
>>> Sorry for the delay in review. Some general questions.
>>>
>>> I remember Wenlei mentioned in the RFC discussion that the overall binary size increase from pseudo probe is ~12% if CFG encoding is added. Is the size increase for object file from pseudo probe about the same as binary? I am asking because the total size of linker input matters for us. Previously we may use fission to separate the debug information from object file so it won't be accounted as linker input.
>>>
>>> According to the description, The address range will be split into multiple ranges by the address of pseudo probe. If some BB somehow doesn't have block type pseudo probe, the addresses in the BB will be mistakenly associated with the pseudo probe located before it in the binary. I don't know whether you saw cases like that. A random thought is we may generate some fake pseudo probes for the BBs which don't have them, as a mark of the BB boundary.
>>
>> Thanks for reviewing this patch!
>>
>> Regarding the binary size, an object file will likely have the same size inflation with an executable since the size of the pseudo probe sections are propositional to the text section. If that is an issue to the linker, we can also perform fission to the pseudo probe data. Pseudo probe encoding is currently done on function (comdat) level (see `MCPseudoProbeSection::Emit` in MCPseudoProbe.cpp`), which means a function always gets a separate real absolute probe encoded in the first place followed by relative address delta encoded for subsequent probes. Therefore fission on function level should be fine. Fission on block level won't work, as you pointed out. Would function-level fission work for you?
>
> Yes, function-level fission works for us. Adding that support later will be helpful.

Cool. It might already be there depending on how fission will be done. The current implementation always emit a relocation from a function's pseudo probe section to the function's text section. The rest of the function's probe will be encoded as a delta from that relocation. We will add more support if this is not sufficient.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91878