[llvm-branch-commits] [openmp] release/18.x: [OpenMP][OMPT] Fix hsa include when building amdgpu/src/rtl.cpp (PR #95484)
jhuber6 wrote: > This doesn't make sense, the ROCm packages may be just badly made by AMD. > > When I temporarily remove `/usr/include/hsa` without uninstalling > `libhsa-runtime-dev` **AND** while using > `-DLIBOMPTARGET_FORCE_DLOPEN_LIBHSA=ON`, I can build LLVM18 without patch. > > With `-DLIBOMPTARGET_FORCE_DLOPEN_LIBHSA=OFF` CMake complains that > `/usr/include/hsa` is missing. > > So it looks like I'm tracking a ROCm packaging bug from AMD side. I don't think AMD handles the actual packaging, that's on the maintainers for your distribution. I'm guessing that the HSA runtime is a separate package from ROCm and gets put somewhere else for whatever reason. I don't have any issues like that with my distribution. > But I still don't get why LLVM doesn't use its own HSA by default and doesn't > use its own headers for enum values used in its own code. It does in the main branch, but I think you're right that the logic causes it to look at the system one first since `hsa/hsa.h` is higher up the search list. Should probably do something about that. https://github.com/llvm/llvm-project/pull/95484 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Drop high discrepancy profiles in matching (PR #95156)
@@ -575,10 +583,16 @@ void preprocessUnreachableBlocks(FlowFunction &Func) {
/// Decide if stale profile matching can be applied for a given function.
/// Currently we skip inference for (very) large instances and for instances
/// having "unexpected" control flow (e.g., having no sink basic blocks).
-bool canApplyInference(const FlowFunction &Func) {
+bool canApplyInference(const FlowFunction &Func,
+ const yaml::bolt::BinaryFunctionProfile &YamlBF,
+ const uint64_t &MatchedBlocks) {
if (Func.Blocks.size() > opts::StaleMatchingMaxFuncSize)
return false;
+ if ((double)(MatchedBlocks) / YamlBF.Blocks.size() <=
aaupov wrote:
It's preferred to use integer math when dealing with options, e.g.:
https://github.com/llvm/llvm-project/blob/67285feffd6708e6db36c11faf95eeab449e797a/bolt/lib/Passes/IndirectCallPromotion.cpp#L584-L586
https://github.com/llvm/llvm-project/pull/95156
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Drop high discrepancy profiles in matching (PR #95156)
@@ -394,7 +400,7 @@ createFlowFunction(const
BinaryFunction::BasicBlockOrderType &BlockOrder) {
void matchWeightsByHashes(BinaryContext &BC,
const BinaryFunction::BasicBlockOrderType
&BlockOrder,
const yaml::bolt::BinaryFunctionProfile &YamlBF,
- FlowFunction &Func) {
+ FlowFunction &Func, uint64_t &MatchedBlocksCount) {
aaupov wrote:
Why not return MatchedBlocksCount instead?
https://github.com/llvm/llvm-project/pull/95156
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Drop high discrepancy profiles in matching (PR #95156)
@@ -394,7 +400,7 @@ createFlowFunction(const
BinaryFunction::BasicBlockOrderType &BlockOrder) {
void matchWeightsByHashes(BinaryContext &BC,
const BinaryFunction::BasicBlockOrderType
&BlockOrder,
const yaml::bolt::BinaryFunctionProfile &YamlBF,
- FlowFunction &Func) {
+ FlowFunction &Func, uint64_t &MatchedBlocksCount) {
shawbyoung wrote:
In the case that we want to build upon our profile discrepancy heuristic, I
think it makes sense to pass in a reference to "something" (variable, struct)
that maintains information related to the heuristic, similarly how to the flow
function is treated by this function. But I don't have strong feelings about
this - if you do I'm happy to change it.
https://github.com/llvm/llvm-project/pull/95156
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [BOLT] Drop high discrepancy profiles in matching (PR #95156)
@@ -394,7 +400,7 @@ createFlowFunction(const
BinaryFunction::BasicBlockOrderType &BlockOrder) {
void matchWeightsByHashes(BinaryContext &BC,
const BinaryFunction::BasicBlockOrderType
&BlockOrder,
const yaml::bolt::BinaryFunctionProfile &YamlBF,
- FlowFunction &Func) {
+ FlowFunction &Func, uint64_t &MatchedBlocksCount) {
aaupov wrote:
I think it makes sense to return the stats as a result of matching. We can
extend the return type into struct in the future if the need arises.
https://github.com/llvm/llvm-project/pull/95156
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [openmp] release/18.x: [OpenMP][OMPT] Fix hsa include when building amdgpu/src/rtl.cpp (PR #95484)
illwieckz wrote:
> I don't think AMD handles the actual packaging.
They do: https://repo.radeon.com
It happens that Ubuntu provides an old 5.2.3 `libhsa-runtime-dev` and it seems
to be a dependency of some package provided by the AMD `repo.radeon.com` own
repository providing ROCm 6.1.2…
> It does in the main branch, but I think you're right that the logic causes it
> to look at the system one first since `hsa/hsa.h` is higher up the search
> list. Should probably do something about that.
Maybe, instead of adding `dynamic_hsa/` directory to include directories and
assuming an ambiguous implicit directory for `hsa.h`, we would not add that
directory and do explicitely:
```c++
#if defined(__has_include)
#if __has_include("dynamic_hsa/hsa.h")
#include "dynamic_hsa/hsa.h"
#include "dynamic_hsa/hsa_ext_amd.h"
#if __has_include("hsa/hsa.h")
#include "hsa/hsa.h"
#include "hsa/hsa_ext_amd.h"
#elif __has_include("hsa.h")
#include "hsa.h"
#include "hsa_ext_amd.h"
#endif
#else
#include "dynamic_hsa/hsa.h"
#include "dynamic_hsa/hsa_ext_amd.h"
#endif
```
https://github.com/llvm/llvm-project/pull/95484
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [openmp] release/18.x: [OpenMP][OMPT] Fix hsa include when building amdgpu/src/rtl.cpp (PR #95484)
illwieckz wrote: The `/usr/include/hsa/hsa.h` file is provided by the stock Ubuntu `libhsa-runtime-dev` package. The `/opt/rocm-6.1.2/include/hsa/hsa.h` file is provided by the AMD `hsa-rocr-dev` package. The AMD `rocm-hip-runtime` package depends on both `hsa-rocr-dev` and `libhsa-runtime-dev`… ``` $ debtree rocm-hip-runtime 2>/dev/null | grep -E -- '-> "hip-runtime-amd|-> "hsa-rocr-dev|-> "hipcc|-> "libamdhip64-dev|-> "libhsa-runtime-dev' "rocm-hip-runtime" -> "hip-runtime-amd" [color=blue,label="(= 6.1.40093.60102-119~22.04)"]; "hip-runtime-amd" -> "hsa-rocr-dev" [color=blue,label="(>= 1.3)"]; "hip-runtime-amd" -> "hipcc" [color=blue]; "hipcc" -> "libamdhip64-dev" [color=blue]; "libamdhip64-dev" -> "libhsa-runtime-dev" [color=blue]; ``` ``` $ apt-get install --reinstall rocm-hip-runtime hip-runtime-amd hsa-rocr-dev hipcc libhsa-runtime-dev Get:1 http://archive.ubuntu.com/ubuntu mantic/universe amd64 hipcc amd64 5.2.3-12 [25,1 kB] Get:2 http://archive.ubuntu.com/ubuntu mantic/universe amd64 libhsa-runtime-dev amd64 5.2.3-5 [73,3 kB] Get:3 https://repo.radeon.com/rocm/apt/6.1.2 jammy/main amd64 hip-runtime-amd amd64 6.1.40093.60102-119~22.04 [27,1 MB] Get:4 https://repo.radeon.com/rocm/apt/6.1.2 jammy/main amd64 hsa-rocr-dev amd64 1.13.0.60102-119~22.04 [102 kB] Get:5 https://repo.radeon.com/rocm/apt/6.1.2 jammy/main amd64 rocm-hip-runtime amd64 6.1.2.60102-119~22.04 [2 042 B] ``` So basically: ``` ─ rocm-hip-runtime 6.1.2.60102-119~22.04 (AMD repository) └ hip-runtime-amd 6.1.40093.60102-119~22.04 (AMD repository) ├ hsa-rocr-dev 1.13.0.60102-119~22.04 (AMD repository) │ └ /opt/rocm-6.1.2/include/hsa/hsa.h └ hipcc 1.0.0.60102-119~22.04 (Ubuntu repository) └ libhsa-runtime-dev 5.0.0-1 (Ubuntu repository) └ /usr/include/hsa/hsa.h ``` This is definitely an AMD packaging issue. https://github.com/llvm/llvm-project/pull/95484 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
