phosek added a comment.

In D132975#3765541 <https://reviews.llvm.org/D132975#3765541>, @Amir wrote:

> Hi Petr, thank you for your comments!
>
> In D132975#3763264 <https://reviews.llvm.org/D132975#3763264>, @phosek wrote:
>
>> This was already on my list of build system features I'd like to implement 
>> and I'm glad someone else is already looking into it, thank you! I have two 
>> high level comments about your approach.
>>
>> The first one is related to the use of Clang build as the training data. I 
>> think that Clang build is both unnecessarily heavyweight, but also not 
>> particularly representative of typical workloads (most Clang users don't use 
>> it to build Clang). Ideally, we would give vendors the flexibility to supply 
>> their own training data. I'd prefer reusing the existing perf-training 
>> <https://github.com/llvm/llvm-project/tree/main/clang/utils/perf-training> 
>> setup to do so. In fact, I'd imagine most vendors would likely use the same 
>> training data for both PGO and BOLT and that use case should be supported.
>
> Agree that perf-training might be useful for vendors. I'll try to enable it 
> in a follow-up diff.
>
> Please note that the target for profile collection is not hardcoded to clang, 
> it's configurable via CLANG_BOLT_INSTRUMENT_PROJECTS and 
> CLANG_BOLT_INSTRUMENT_TARGETS. Right now it's the llvm/not tool (the smallest 
> possible).
>
>> The second one is related to applicability. I don't think this mechanism 
>> should be limited only to Clang. Ideally, it should be possible to 
>> instrument and optimize other tools in the toolchain distribution as well; 
>> LLD is likely going to be the most common one after Clang.
>
> I thought about it, and I think we can accommodate optimizing arbitrary 
> targets by providing an interface to instrument specified target(s) via 
> `-DBOLT_INSTRUMENT_TARGETS`. For each of the target binaries, CMake would 
> create targets like `bolt-instrument-$TARGET` and `bolt-optimize-$TARGET`. 
> For `bolt-instrument-$TARGET`, BOLT would instrument the target binary, 
> placing instrumented binary next to the original one (e.g. 
> `$target-bolt.inst`). End users would use those instrumented binaries on 
> representative workloads to collect the profile. For `bolt-optimize-$TARGET`, 
> BOLT would post-process the profiles and create optimized binary 
> (`$target-bolt`).
>
> I appreciate your suggestions. Do you think we can move incrementally from 
> this diff towards more general uses in follow-up diffs?

That's fine with me. Do you envision replacing the use of LLVM build for 
training with perf-training or supporting both? I'd lean towards the former for 
simplicity but would be curious to hear about your use cases and plans.



================
Comment at: clang/CMakeLists.txt:881
 
+if (CLANG_BOLT_INSTRUMENT)
+  set(CLANG_PATH ${LLVM_RUNTIME_OUTPUT_INTDIR}/clang)
----------------
We could consider moving this block to a separate file which would then be 
included here since this file is already getting pretty large and the logic in 
this block is self-contained. That could be done in a follow up change though.


================
Comment at: clang/CMakeLists.txt:955
+    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+    COMMAND sh -c "$<TARGET_FILE:merge-fdata> prof.fdata.* -o prof.fdata"
+    COMMENT "Preparing BOLT profile"
----------------
I'd like to avoid dependency on shell to make this compatible with Windows. Can 
we move this logic into a Python script akin to 
https://github.com/llvm/llvm-project/blob/607f14d9605da801034e7119c297c3f58ebce603/clang/utils/perf-training/perf-helper.py?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132975

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to