Skip to content

[ClangOffloadBundler] make hipv4 and hip compatible #91637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 9, 2024

Conversation

yxsamliu
Copy link
Collaborator

@yxsamliu yxsamliu commented May 9, 2024

The distinction between the hip and hipv4 offload kinds is historically based. Originally, these designations might have indicated different versions of the code object ABI (Application Binary Interface). However, as the system has evolved, the ABI version is now embedded directly within the code object itself, making these historical distinctions irrelevant during the unbundling process. Consequently, hip and hipv4 are treated as compatible in current implementations, facilitating interchangeable handling of code objects without differentiation based on offload kind. This change streamlines code management within the ecosystem.

@yxsamliu yxsamliu requested review from Artem-B, jhuber6 and lamb-j May 9, 2024 18:17
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels May 9, 2024
@llvmbot
Copy link
Member

llvmbot commented May 9, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Yaxun (Sam) Liu (yxsamliu)

Changes

The distinction between the hip and hipv4 offload kinds is historically based. Originally, these designations might have indicated different versions of the code object ABI (Application Binary Interface). However, as the system has evolved, the ABI version is now embedded directly within the code object itself, making these historical distinctions irrelevant during the unbundling process. Consequently, hip and hipv4 are treated as compatible in current implementations, facilitating interchangeable handling of code objects without differentiation based on offload kind. This change streamlines code management within the ecosystem.


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

3 Files Affected:

  • (modified) clang/docs/ClangOffloadBundler.rst (+10-2)
  • (modified) clang/lib/Driver/OffloadBundler.cpp (+4-1)
  • (modified) clang/test/Driver/clang-offload-bundler.c (+7)
diff --git a/clang/docs/ClangOffloadBundler.rst b/clang/docs/ClangOffloadBundler.rst
index 515e6c00a3b80..3c241027d405c 100644
--- a/clang/docs/ClangOffloadBundler.rst
+++ b/clang/docs/ClangOffloadBundler.rst
@@ -245,7 +245,7 @@ Where:
                     object as a data section with the name ``.hip_fatbin``.
 
       hipv4         Offload code object for the HIP language. Used for AMD GPU
-                    code objects with at least ABI version V4 when the
+                    code objects with at least ABI version V4 and above when the
                     ``clang-offload-bundler`` is used to create a *fat binary*
                     to be loaded by the HIP runtime. The fat binary can be
                     loaded directly from a file, or be embedded in the host code
@@ -254,6 +254,14 @@ Where:
       openmp        Offload code object for the OpenMP language extension.
       ============= ==============================================================
 
+Note: The distinction between the `hip` and `hipv4` offload kinds is historically based.
+Originally, these designations might have indicated different versions of the
+code object ABI. However, as the system has evolved, the ABI version is now embedded
+directly within the code object itself, making these historical distinctions irrelevant
+during the unbundling process. Consequently, `hip` and `hipv4` are treated as compatible
+in current implementations, facilitating interchangeable handling of code objects
+without differentiation based on offload kind.
+
 **target-triple**
     The target triple of the code object. See `Target Triple
     <https://clang.llvm.org/docs/CrossCompilation.html#target-triple>`_.
@@ -295,7 +303,7 @@ Compatibility Rules for Bundle Entry ID
   A code object, specified using its Bundle Entry ID, can be loaded and
   executed on a target processor, if:
 
-  * Their offload kinds are the same.
+  * Their offload kinds are the same or comptible.
   * Their target triples are compatible.
   * Their Target IDs are compatible as defined in :ref:`compatibility-target-id`.
 
diff --git a/clang/lib/Driver/OffloadBundler.cpp b/clang/lib/Driver/OffloadBundler.cpp
index 8cc82a0ee7168..191d108e9b739 100644
--- a/clang/lib/Driver/OffloadBundler.cpp
+++ b/clang/lib/Driver/OffloadBundler.cpp
@@ -113,8 +113,11 @@ bool OffloadTargetInfo::isOffloadKindValid() const {
 
 bool OffloadTargetInfo::isOffloadKindCompatible(
     const StringRef TargetOffloadKind) const {
-  if (OffloadKind == TargetOffloadKind)
+  if ((OffloadKind == TargetOffloadKind) ||
+      (OffloadKind == "hip" && TargetOffloadKind == "hipv4") ||
+      (OffloadKind == "hipv4" && TargetOffloadKind == "hip"))
     return true;
+
   if (BundlerConfig.HipOpenmpCompatible) {
     bool HIPCompatibleWithOpenMP = OffloadKind.starts_with_insensitive("hip") &&
                                    TargetOffloadKind == "openmp";
diff --git a/clang/test/Driver/clang-offload-bundler.c b/clang/test/Driver/clang-offload-bundler.c
index e492da31abb74..bb609648fcd53 100644
--- a/clang/test/Driver/clang-offload-bundler.c
+++ b/clang/test/Driver/clang-offload-bundler.c
@@ -505,6 +505,13 @@
 // RUN:   -output=%t.res.tgt1 -input=%t.hip.bundle.bc -unbundle 2>&1 | FileCheck %s -check-prefix=NOGFX906
 // NOGFX906: error: Can't find bundles for hip-amdgcn-amd-amdhsa--gfx906
 
+//
+// Check hip and hipv4 are compatible as offload kind.
+//
+// RUN: clang-offload-bundler -type=o -targets=hip-amdgcn-amd-amdhsa--gfx90a -input=%t.tgt1 -output=%t.bundle3.o
+// RUN: clang-offload-bundler -type=o -targets=hipv4-amdgcn-amd-amdhsa--gfx90a:sramecc-:xnack+ -output=%t.res.tgt1 -input=%t.bundle3.o -unbundle
+// RUN: diff %t.tgt1 %t.res.tgt1
+
 //
 // Check archive unbundling
 //

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some code in the clang-linker-wrapper that creates the offloadbundler format for HIP offloading. I think it and the tests use hipv4 which we could presumably remove now?

The distinction between the hip  and hipv4  offload kinds is historically based.
Originally, these designations might have indicated different versions of the
code object ABI (Application Binary Interface). However, as the system has
evolved, the ABI version is now embedded directly within the code object itself,
making these historical distinctions irrelevant during the unbundling process.
Consequently, hip and hipv4 are treated as compatible in current implementations,
facilitating interchangeable handling of code objects without differentiation based
on offload kind. This change streamlines code management within the  ecosystem.
@yxsamliu
Copy link
Collaborator Author

yxsamliu commented May 9, 2024

There's some code in the clang-linker-wrapper that creates the offloadbundler format for HIP offloading. I think it and the tests use hipv4 which we could presumably remove now?

yes. I updated linker wrapper to use hip- instead of hipv4-

@yxsamliu yxsamliu merged commit ca39175 into llvm:main May 9, 2024
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request May 21, 2024
This reflects upstream changes which make hip and hipv4
compatible: llvm#91637

Change-Id: Ia00f0cabce442c12b88ae2007045869243602fec
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request May 25, 2024
The distinction between the hip and hipv4 offload kinds is historically
based. Originally, these designations might have indicated different
versions of the code object ABI (Application Binary Interface). However,
as the system has evolved, the ABI version is now embedded directly
within the code object itself, making these historical distinctions
irrelevant during the unbundling process. Consequently, hip and hipv4
are treated as compatible in current implementations, facilitating
interchangeable handling of code objects without differentiation based
on offload kind. This change streamlines code management within the
ecosystem.

Fixes: SWDEV-460869

Change-Id: I605647f12ef852a14afc13960cc765c5f0b4f12b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants