Fix to account for multiple ISA enumeration

[hidekisaito]

When multiple ISAs are enumerated per agent, existing code picked up the last one. Incumbent is the first one and thus the fixed code picks up the first one. This fix is expected to be short-lived --- to be followed by code object version 6 generic ISA support.

[llvmbot]

@llvm/pr-subscribers-offload

@llvm/pr-subscribers-backend-amdgpu

Author: None (hidekisaito)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/118676.diff

1 Files Affected:

• (modified) offload/plugins-nextgen/amdgpu/src/rtl.cpp (+1-1)

diff --git a/offload/plugins-nextgen/amdgpu/src/rtl.cpp b/offload/plugins-nextgen/amdgpu/src/rtl.cpp
index 22c8079ab5812f..d74e65d4165679 100644
--- a/offload/plugins-nextgen/amdgpu/src/rtl.cpp
+++ b/offload/plugins-nextgen/amdgpu/src/rtl.cpp
@@ -207,7 +207,7 @@ Expected<std::string> getTargetTripleAndFeatures(hsa_agent_t Agent) {
     llvm::StringRef TripleTarget(ISAName.begin(), Length);
     if (TripleTarget.consume_front("amdgcn-amd-amdhsa"))
       Target = TripleTarget.ltrim('-').rtrim('\0').str();
-    return HSA_STATUS_SUCCESS;
+    return HSA_STATUS_INFO_BREAK;
   });
   if (Err)
     return Err;

[jhuber6]

Good short-term fix, long term we just need to handle multiple of these and push them back. Then we can scan all the ISAs from least generic to most generic through some complicated lookup table.

[shiltian]

Good short-term fix, long term we just need to handle multiple of these and push them back. Then we can scan all the ISAs from least generic to most generic through some complicated lookup table.

This is not a short term fix. The iteration always starts from specific to generic, so there must be only one match. It is just code error not stopping it on the first match.

I also don't think it's a good idea to maintain a match table at OpenMP level.

[shiltian]

Also, this PR doesn't have proper prefix, such as [Offload][AMDGPU].

[jhuber6]

Also, this PR doesn't have proper prefix, such as [Offload][AMDGPU].

Forgot to fix that before I hit merge.

This is not a short term fix. The iteration always starts from specific to generic, so there must be only one match. It is just code error not stopping it on the first match.

That's not what the code is doing, it's copying out the first result to then do a comparison. Maybe we could move the check inside of the iterate callback and then break when we first find a match?

I also don't think it's a good idea to maintain a match table at OpenMP level.

Not the OpenMP level, we need one at the ELF flags level.

[shiltian]

Not the OpenMP level, we need one at the ELF flags level.

The generic ELF has to match generic ISA provided by HSA. There is no need to do that at the ELF flag level. For example, if the ELF is gfx10-3-generic, it should not work with gfx1030, at least not based on the design of HSA.

That's not what the code is doing, it's copying out the first result to then do a comparison. Maybe we could move the check inside of the iterate callback and then break when we first find a match?

Right, that should be the right approach IMHO.