[PATCH] D105217: [LLD] Adding support for RELA for CG Profile.

Fri Jul 2 11:12:20 PDT 2021

MaskRay added a comment.

In D105217#2855739 <https://reviews.llvm.org/D105217#2855739>, @wenlei wrote:

> In D105217#2851379 <https://reviews.llvm.org/D105217#2851379>, @MaskRay wrote:
>
>> First, `.llvm.call-graph-profile` is only emitted by -fprofile-use= and -fprofile-sample-use=, instrumentation PGO and sample PGO.
>> The use case is very specific.
>
> Not sure if I follow this. If what you meant is we don't need to have good support for it since this call graph profile is not used by everyone...

No.

> I'm afraid I don't agree: 1. We used this broadly for hundreds of workloads. 2. Toolchains have many features most of which aren't used by many, but as long as those who use them are willing to maintain and improve it, why would we want to block such work just because it's not a mainstream feature?

I meant: some features have some particular requirement. We don't want to add support for something nobody uses.
For this point, I saw this patch so I knew you are going to use it. If it didn't add 16-bytes to each InputFile I would be ok with it.
However, it has such overhead so I need to balance the needs with the costs and want to ensure you would not add the cost if you could fix the build system internally.

> In D105217#2852279 <https://reviews.llvm.org/D105217#2852279>, @jhenderson wrote:
>
>> I don't think the additional complexity is all that much, especially given there's an actual use-case for it. It'd be different if the case was some hypothetical situation, but it isn't. Not all systems have all of LLVM installed as their toolchain, so forcing people away from using GNU tools seems like the wrong approach to me.
>
> Exactly. As mentioned above, we have precedence for supporting some level of compatibility. Insisting on simplicity and closed toolchain would do more harm than good.

See my previous comment. My main concern is the size overhead to InputFile. I am thinking we are paying too much for a relatively less useful feature.

The `fatal` issue can be addressed by, e.g. emitting a warning instead or ignoring the section completely.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105217