[PATCH] D106775: [MCA] Moving the target specific CustomBehaviour impl. from /tools/llvm-mca/ to /lib/Target/.

Patrick Holland via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jul 25 14:43:36 PDT 2021


holland11 created this revision.
holland11 added reviewers: andreadb, tstellar, qcolombet, foad.
Herald added subscribers: dexonsmith, kerbowa, gbedwell, hiraditya, tpr, mgorny, nhaehnle, jvesely, arsenm.
holland11 requested review of this revision.
Herald added a project: LLVM.

This patch is related to https://reviews.llvm.org/D104149 and https://reviews.llvm.org/D104730 . The first patch was introducing the `CustomBehaviour` class to `llvm-mca`. To start, target specific `CBs` were meant to be placed within the `llvm-mca` directory. Since the class is only used by `llvm-mca`, this made sense and was the cleanest and most intuitive place to put them.

However, in the second patch, I attempted to implement some custom modelling for `AMDGPU`. This modelling required the use of some backend functions and symbols. This works fine for static builds and for shared builds that are not on linux. However, when building with shared libs on linux, there were build errors because the backend symbols were not accessible from the `AMDGPUCB`. For more details on this, you can refer to the most recent comments within the second diff linked above.

To address these errors, I restructured the target specific `CB` implementations to be placed within the `/lib/Target/<Target-Name>/MCA` directory. (This `MCA` directory is new.) Since the implementations are now a part of the target's backend, they need to be initialized and registered similar to how many other target specific classes work (such as `AsmParser`). To accomplish this, I had to add some code to several cmake files throughout `llvm`. This is my first time working with general llvm config stuff so if I made any mistakes, hopefully someone can spot them and help me correct them.

This patch keeps the `AMDGPUCB` implementation empty. If/when this patch gets approved and pushed, I will submit a diff for the `AMDGPUCB` implementation (which is the same implementation seen in the second diff linked above).

I have tested several builds and run `ninja check all` on them on both macOS and linux. I have tested them with the future `AMDGPUCB` implementation as well to make sure that this solution does in fact fix the original error regarding unresolved symbols when building with shared libs on linux.

If anybody sees any issues or has any suggestions for ways that this can be done better, I'd be happy to tweak or modify this patch.

Thanks for your time!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106775

Files:
  llvm/CMakeLists.txt
  llvm/cmake/modules/LLVM-Config.cmake
  llvm/docs/CommandGuide/llvm-mca.rst
  llvm/include/llvm/Config/TargetMCAs.def.in
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/include/llvm/MCA/CustomBehaviour.h
  llvm/include/llvm/Support/TargetRegistry.h
  llvm/include/llvm/Support/TargetSelect.h
  llvm/lib/Target/AMDGPU/CMakeLists.txt
  llvm/lib/Target/AMDGPU/MCA/AMDGPUCustomBehaviour.cpp
  llvm/lib/Target/AMDGPU/MCA/AMDGPUCustomBehaviour.h
  llvm/lib/Target/AMDGPU/MCA/CMakeLists.txt
  llvm/tools/llvm-mca/CMakeLists.txt
  llvm/tools/llvm-mca/CodeRegionGenerator.h
  llvm/tools/llvm-mca/lib/AMDGPU/AMDGPUCustomBehaviour.cpp
  llvm/tools/llvm-mca/lib/AMDGPU/AMDGPUCustomBehaviour.h
  llvm/tools/llvm-mca/lib/AMDGPU/CMakeLists.txt
  llvm/tools/llvm-mca/lib/CMakeLists.txt
  llvm/tools/llvm-mca/llvm-mca.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D106775.361541.patch
Type: text/x-patch
Size: 26007 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210725/175c93fe/attachment.bin>


More information about the llvm-commits mailing list