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

Fri Jul 2 11:01:16 PDT 2021

wenlei added a comment.

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... 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?

> For sample PGO/BOLT there are other requirements from an external tool converting linux-perf data to a profile format recognized by llvm-project.
> I can imagine that BOLT may have more requirements on LLVM tooling.
> So I don't see why requiring llvm-strip will be another hindrance.

This is more of a build system issue - requiring all gnu tools to be replaced by llvm tools in any large organization is going to be very difficult. I think it's reasonable to support some level of compatibility if the complexity isn't high.

In fact, there're many precedences for that. In LLD, we have a list of silently ignored switches for compatibility. We could insist build system changes for all instead of accommodate them in toolchain, but we favored compatibility.

You mentioned extra complexity, while I agree that if possible simpler implementation and tighter contract is better, but the trade off here between very minor complexity and compatibility seem not very different from those silently ignored switches.

> Also keep in mind that GNU binutils doesn't recognize the section type SHT_LLVM_*. (Folks have added dumping support to llvm-readobj.)
>
>> Second is how caching of object files works within the build system. It allows flexibility for projects to re-use objects with and without debug information.
>
> If linker input size is not a concern, you may use `ld -S` instead of `strip -S` on .o files.

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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105217