[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j added a comment.

In D145770#4220246 , @mdtoguchi wrote:

> @lamb-j - is it expected for any bundled objects created before your change 
> without the explicit env field to be able to be unbundled?  Newly generated 
> bundles work as expected given similar `-target` values, but older generated 
> binaries fail to unbundle the target given equivalent commands.  Is it 
> possible to provide the ability to do so?

That should still be supported. The target triple for the old bundle should be 
converted to the new format (and compared against the Target-ID that was also 
converted to the new format). Can you provide an example to recreate the 
failure? I just tried one example locally and didn't hit any failures:

  old-clang -c --offload-arch=gfx906 -emit-llvm -fgpu-rdc --gpu-bundle-output 
square.hip
  new-clang-offload-bundler -unbundle -type=bc 
-targets=hip-amdgcn-amd-amdhsa-gfx906 -input=square.bc 
-output=square-hip-gfx906.bc -allow-missing-bundles 
-debug-only=CodeObjectCompatibility
  
  Compatible: Exact match:[CodeObject: hip-amdgcn-amd-amdhsa--gfx906]   
  :   [Target: hip-amdgcn-amd-amdhsa--gfx906]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D145770#4220246 , @mdtoguchi wrote:

> @lamb-j - is it expected for any bundled objects created before your change 
> without the explicit env field to be able to be unbundled?  Newly generated 
> bundles work as expected given similar `-target` values, but older generated 
> binaries fail to unbundle the target given equivalent commands.  Is it 
> possible to provide the ability to do so?

what is the target used by the old bundle? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Michael D Toguchi via Phabricator via cfe-commits]

mdtoguchi added a comment.

@lamb-j - is it expected for any bundled objects created before your change 
without the explicit env field to be able to be unbundled?  Newly generated 
bundles work as expected given similar `-target` values, but older generated 
binaries fail to unbundle the target given equivalent commands.  Is it possible 
to provide the ability to do so?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j added a comment.
Herald added a subscriber: jplehr.

Needed to update this after landing with two quick patch fixes:

  [clang-offload-bundler] Fix test failures and document typo (65fb636bd462)
  
  [clang-offload-bundler] Fix error with regex in bundler test (f3b991202689)

There were also two quick fixes by others that overlapped but got in before my 
fixes:

  Mark test modified in e48ae0d as XFAIL for PS4/PS5 until the author ... 
(768211f48f2d)
  
  clang/test/Driver/clang-offload-bundler-standardize.c REQUIRES asserts. 
(fixup D145770) (d4a4d0d79127)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

In D145770#4195018 , @dyung wrote:

> @lamb-j the test clang-offload-bundler.c is failing on the PS4 linux builder, 
> can you take a look?
>
> https://lab.llvm.org/buildbot/#/builders/139/builds/37475
>
> I don't know if it matters, but this bot only builds the x86_64 target.

To unblock our bots, I've marked the test as XFAIL for PS4/PS5 in 
768211f48f2d52fdd555a07f3dae8fbf1def88cc 
, please 
investigate when you can.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

@lamb-j the test clang-offload-bundler.c is failing on the PS4 linux builder, 
can you take a look?

https://lab.llvm.org/buildbot/#/builders/139/builds/37475

I don't know if it matters, but this bot only builds the x86_64 target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Jacob Lambert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe48ae0dbd80d: [clang-offload-bundler] Standardize TargetID 
field for bundler (authored by lamb-j).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

Files:
  clang/docs/ClangOffloadBundler.rst
  clang/lib/Driver/OffloadBundler.cpp
  clang/test/Driver/clang-offload-bundler-asserts-on.c
  clang/test/Driver/clang-offload-bundler-standardize.c
  clang/test/Driver/clang-offload-bundler.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -313,6 +313,8 @@
   llvm::DenseSet ParsedTargets;
   // Map {offload-kind}-{triple} to target IDs.
   std::map> TargetIDs;
+  // Standardize target names to include env field
+  std::vector StandardizedTargetNames;
   for (StringRef Target : TargetNames) {
 if (ParsedTargets.contains(Target)) {
   reportError(createStringError(errc::invalid_argument,
@@ -324,6 +326,8 @@
 bool KindIsValid = OffloadInfo.isOffloadKindValid();
 bool TripleIsValid = OffloadInfo.isTripleValid();
 
+StandardizedTargetNames.push_back(OffloadInfo.str());
+
 if (!KindIsValid || !TripleIsValid) {
   SmallVector Buf;
   raw_svector_ostream Msg(Buf);
@@ -348,6 +352,9 @@
 
 ++Index;
   }
+
+  BundlerConfig.TargetNames = StandardizedTargetNames;
+
   for (const auto  : TargetIDs) {
 if (auto ConflictingTID =
 clang::getConflictTargetIDCombination(TargetID.second)) {
Index: clang/test/Driver/clang-offload-bundler.c
===
--- clang/test/Driver/clang-offload-bundler.c
+++ clang/test/Driver/clang-offload-bundler.c
@@ -1,5 +1,4 @@
 // REQUIRES: x86-registered-target
-// REQUIRES: powerpc-registered-target
 // UNSUPPORTED: target={{.*}}-darwin{{.*}}, target={{.*}}-aix{{.*}}
 
 //
@@ -297,11 +296,11 @@
 
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle3.o -### 2>&1 \
 // RUN: | FileCheck %s -DHOST=%itanium_abi_triple -DINOBJ1=%t.o -DINOBJ2=%t.tgt1 -DINOBJ3=%t.tgt2 -DOUTOBJ=%t.bundle3.o --check-prefix CK-OBJ-CMD
-// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=readonly,exclude" "--" "[[INOBJ1]]" "[[OUTOBJ]]"
+// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu-=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu-=readonly,exclude" "--" "[[INOBJ1]]" "[[OUTOBJ]]"
 
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle3.o -### 2>&1 \
 // RUN: | FileCheck %s -DHOST=%itanium_abi_triple -DINOBJ1=%t.o -DINOBJ2=%t.tgt1 -DINOBJ3=%t.tgt2 -DOUTOBJ=%t.bundle3.o --check-prefix CK-OBJ-CMD-INPUTS
-// CK-OBJ-CMD-INPUTS: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=readonly,exclude" "--" "[[INOBJ1]]" "[[OUTOBJ]]"
+// CK-OBJ-CMD-INPUTS: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=readonly,exclude" 

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j updated this revision to Diff 505227.
lamb-j added a comment.

Updating commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

Files:
  clang/docs/ClangOffloadBundler.rst
  clang/lib/Driver/OffloadBundler.cpp
  clang/test/Driver/clang-offload-bundler-asserts-on.c
  clang/test/Driver/clang-offload-bundler-standardize.c
  clang/test/Driver/clang-offload-bundler.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -313,6 +313,8 @@
   llvm::DenseSet ParsedTargets;
   // Map {offload-kind}-{triple} to target IDs.
   std::map> TargetIDs;
+  // Standardize target names to include env field
+  std::vector StandardizedTargetNames;
   for (StringRef Target : TargetNames) {
 if (ParsedTargets.contains(Target)) {
   reportError(createStringError(errc::invalid_argument,
@@ -324,6 +326,8 @@
 bool KindIsValid = OffloadInfo.isOffloadKindValid();
 bool TripleIsValid = OffloadInfo.isTripleValid();
 
+StandardizedTargetNames.push_back(OffloadInfo.str());
+
 if (!KindIsValid || !TripleIsValid) {
   SmallVector Buf;
   raw_svector_ostream Msg(Buf);
@@ -348,6 +352,9 @@
 
 ++Index;
   }
+
+  BundlerConfig.TargetNames = StandardizedTargetNames;
+
   for (const auto  : TargetIDs) {
 if (auto ConflictingTID =
 clang::getConflictTargetIDCombination(TargetID.second)) {
Index: clang/test/Driver/clang-offload-bundler.c
===
--- clang/test/Driver/clang-offload-bundler.c
+++ clang/test/Driver/clang-offload-bundler.c
@@ -1,5 +1,4 @@
 // REQUIRES: x86-registered-target
-// REQUIRES: powerpc-registered-target
 // UNSUPPORTED: target={{.*}}-darwin{{.*}}, target={{.*}}-aix{{.*}}
 
 //
@@ -297,11 +296,11 @@
 
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle3.o -### 2>&1 \
 // RUN: | FileCheck %s -DHOST=%itanium_abi_triple -DINOBJ1=%t.o -DINOBJ2=%t.tgt1 -DINOBJ3=%t.tgt2 -DOUTOBJ=%t.bundle3.o --check-prefix CK-OBJ-CMD
-// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=readonly,exclude" "--" "[[INOBJ1]]" "[[OUTOBJ]]"
+// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu-=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu-=readonly,exclude" "--" "[[INOBJ1]]" "[[OUTOBJ]]"
 
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle3.o -### 2>&1 \
 // RUN: | FileCheck %s -DHOST=%itanium_abi_triple -DINOBJ1=%t.o -DINOBJ2=%t.tgt1 -DINOBJ3=%t.tgt2 -DOUTOBJ=%t.bundle3.o --check-prefix CK-OBJ-CMD-INPUTS
-// CK-OBJ-CMD-INPUTS: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=readonly,exclude" "--" "[[INOBJ1]]" "[[OUTOBJ]]"
+// CK-OBJ-CMD-INPUTS: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu-=[[INOBJ3]]" 

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Please update the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j added a comment.

In D145770#4190901 , @scott.linder 
wrote:

> Is there a corresponding document update to go along with this? If consumers 
> of the bundles will begin to rely on this it should be documented that it is 
> guaranteed by the producer.

Good point, documentation udpated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j updated this revision to Diff 504893.
lamb-j added a comment.

Adding note to documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

Files:
  clang/docs/ClangOffloadBundler.rst
  clang/lib/Driver/OffloadBundler.cpp
  clang/test/Driver/clang-offload-bundler-asserts-on.c
  clang/test/Driver/clang-offload-bundler-standardize.c
  clang/test/Driver/clang-offload-bundler.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -313,6 +313,8 @@
   llvm::DenseSet ParsedTargets;
   // Map {offload-kind}-{triple} to target IDs.
   std::map> TargetIDs;
+  // Standardize target names to include env field
+  std::vector StandardizedTargetNames;
   for (StringRef Target : TargetNames) {
 if (ParsedTargets.contains(Target)) {
   reportError(createStringError(errc::invalid_argument,
@@ -324,6 +326,8 @@
 bool KindIsValid = OffloadInfo.isOffloadKindValid();
 bool TripleIsValid = OffloadInfo.isTripleValid();
 
+StandardizedTargetNames.push_back(OffloadInfo.str());
+
 if (!KindIsValid || !TripleIsValid) {
   SmallVector Buf;
   raw_svector_ostream Msg(Buf);
@@ -348,6 +352,9 @@
 
 ++Index;
   }
+
+  BundlerConfig.TargetNames = StandardizedTargetNames;
+
   for (const auto  : TargetIDs) {
 if (auto ConflictingTID =
 clang::getConflictTargetIDCombination(TargetID.second)) {
Index: clang/test/Driver/clang-offload-bundler.c
===
--- clang/test/Driver/clang-offload-bundler.c
+++ clang/test/Driver/clang-offload-bundler.c
@@ -1,5 +1,4 @@
 // REQUIRES: x86-registered-target
-// REQUIRES: powerpc-registered-target
 // UNSUPPORTED: target={{.*}}-darwin{{.*}}, target={{.*}}-aix{{.*}}
 
 //
@@ -297,11 +296,11 @@
 
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle3.o -### 2>&1 \
 // RUN: | FileCheck %s -DHOST=%itanium_abi_triple -DINOBJ1=%t.o -DINOBJ2=%t.tgt1 -DINOBJ3=%t.tgt2 -DOUTOBJ=%t.bundle3.o --check-prefix CK-OBJ-CMD
-// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=readonly,exclude" "--" "[[INOBJ1]]" "[[OUTOBJ]]"
+// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu-=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu-=readonly,exclude" "--" "[[INOBJ1]]" "[[OUTOBJ]]"
 
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle3.o -### 2>&1 \
 // RUN: | FileCheck %s -DHOST=%itanium_abi_triple -DINOBJ1=%t.o -DINOBJ2=%t.tgt1 -DINOBJ3=%t.tgt2 -DOUTOBJ=%t.bundle3.o --check-prefix CK-OBJ-CMD-INPUTS
-// CK-OBJ-CMD-INPUTS: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=readonly,exclude" "--" "[[INOBJ1]]" "[[OUTOBJ]]"
+// CK-OBJ-CMD-INPUTS: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu-=[[INOBJ3]]" 

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

Is there a corresponding document update to go along with this? If consumers of 
the bundles will begin to rely on this it should be documented that it is 
guaranteed by the producer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j marked an inline comment as done.
lamb-j added inline comments.



Comment at: clang/test/Driver/clang-offload-bundler.c:2
 // REQUIRES: x86-registered-target
-// REQUIRES: powerpc-registered-target
 // UNSUPPORTED: target={{.*}}-darwin{{.*}}, target={{.*}}-aix{{.*}}

Can most systems cross-compile for powerpc? This test does a lot of testing, 
seems like it would be nice to test it on other systems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j updated this revision to Diff 504310.
lamb-j added a comment.
Herald added a reviewer: alexander-shaposhnikov.

Adding another test, updating existing large test, and switching from ABI
to env


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

Files:
  clang/lib/Driver/OffloadBundler.cpp
  clang/test/Driver/clang-offload-bundler-asserts-on.c
  clang/test/Driver/clang-offload-bundler-standardize.c
  clang/test/Driver/clang-offload-bundler.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -313,6 +313,8 @@
   llvm::DenseSet ParsedTargets;
   // Map {offload-kind}-{triple} to target IDs.
   std::map> TargetIDs;
+  // Standardize target names to include env field
+  std::vector StandardizedTargetNames;
   for (StringRef Target : TargetNames) {
 if (ParsedTargets.contains(Target)) {
   reportError(createStringError(errc::invalid_argument,
@@ -324,6 +326,8 @@
 bool KindIsValid = OffloadInfo.isOffloadKindValid();
 bool TripleIsValid = OffloadInfo.isTripleValid();
 
+StandardizedTargetNames.push_back(OffloadInfo.str());
+
 if (!KindIsValid || !TripleIsValid) {
   SmallVector Buf;
   raw_svector_ostream Msg(Buf);
@@ -348,6 +352,9 @@
 
 ++Index;
   }
+
+  BundlerConfig.TargetNames = StandardizedTargetNames;
+
   for (const auto  : TargetIDs) {
 if (auto ConflictingTID =
 clang::getConflictTargetIDCombination(TargetID.second)) {
Index: clang/test/Driver/clang-offload-bundler.c
===
--- clang/test/Driver/clang-offload-bundler.c
+++ clang/test/Driver/clang-offload-bundler.c
@@ -1,5 +1,4 @@
 // REQUIRES: x86-registered-target
-// REQUIRES: powerpc-registered-target
 // UNSUPPORTED: target={{.*}}-darwin{{.*}}, target={{.*}}-aix{{.*}}
 
 //
@@ -297,11 +296,11 @@
 
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle3.o -### 2>&1 \
 // RUN: | FileCheck %s -DHOST=%itanium_abi_triple -DINOBJ1=%t.o -DINOBJ2=%t.tgt1 -DINOBJ3=%t.tgt2 -DOUTOBJ=%t.bundle3.o --check-prefix CK-OBJ-CMD
-// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=readonly,exclude" "--" "[[INOBJ1]]" "[[OUTOBJ]]"
+// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu-=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu-=readonly,exclude" "--" "[[INOBJ1]]" "[[OUTOBJ]]"
 
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle3.o -### 2>&1 \
 // RUN: | FileCheck %s -DHOST=%itanium_abi_triple -DINOBJ1=%t.o -DINOBJ2=%t.tgt1 -DINOBJ3=%t.tgt2 -DOUTOBJ=%t.bundle3.o --check-prefix CK-OBJ-CMD-INPUTS
-// CK-OBJ-CMD-INPUTS: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=readonly,exclude" "--" "[[INOBJ1]]" "[[OUTOBJ]]"
+// CK-OBJ-CMD-INPUTS: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]-=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu-=readonly,exclude" 

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j added a comment.

In D145770#4186142 , @yaxunl wrote:

> In D145770#4185658 , @lamb-j wrote:
>
>> In D145770#4184996 , @yaxunl wrote:
>>
>>> The description needs fix. "ABI field" should be "environment component".
>>>
>>> Also, we need a clang-offload-bundler test which bundles with a 
>>> non-canonical triple and unbundles with a canonical triple and vice versa.
>>
>> I'm not sure if we can use Triple.normalize(). Here's what I got from a 
>> small test:
>>
>>   amdgcn-amd-amdhsa  ->  amdgcn-amd-amdhsa (no env field added)
>>   amdgcn-amd-amdhsa- ->  amdgcn-amd-amdhsa-unknown (empty env changed to 
>> unknown)
>>
>> Also are we sure ABI is incorrect? That's what is used here:
>>
>> https://clang.llvm.org/docs/CrossCompilation.html
>>
>> (I'm happy to change to environment, just double checking)
>
> https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/TargetParser/Triple.h
>  only talks about "environment", which seems to be more accurate and 
> up-to-date.
>
> It is unfortunate that we have used our own normalization (empty string for 
> unspecified environment instead of "unknown") for triple. Looks like we have 
> to keep that for now. Ideally, HIP runtime should accept 
> "amdgcn-amd-amdhsa-unknown" in addition to "amdgcn-amd-amdhsa--" in fat 
> binary, then we could switch to the LLVM Triple normalization. But that 
> requires runtime change first. Probably we could do that in future.

It may be fine to use "unknown" instead of an empty string, I didn't test that. 
It's just that the Triple.normalize() doesn't add anything for the env field if 
there wasn't anything there to begin with. That is, it doesn't convert a 
3-field triple to a 4-field triple (it only converts a 4-field with empty 
string env to 4-field "unknown" env").

And sounds good about abi vs. env, maybe I'll add another patch to update 
CrossCompilation.html so it's more consistent with Triple.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D145770#4185658 , @lamb-j wrote:

> In D145770#4184996 , @yaxunl wrote:
>
>> The description needs fix. "ABI field" should be "environment component".
>>
>> Also, we need a clang-offload-bundler test which bundles with a 
>> non-canonical triple and unbundles with a canonical triple and vice versa.
>
> I'm not sure if we can use Triple.normalize(). Here's what I got from a small 
> test:
>
>   amdgcn-amd-amdhsa  ->  amdgcn-amd-amdhsa (no env field added)
>   amdgcn-amd-amdhsa- ->  amdgcn-amd-amdhsa-unknown (empty env changed to 
> unknown)
>
> Also are we sure ABI is incorrect? That's what is used here:
>
> https://clang.llvm.org/docs/CrossCompilation.html
>
> (I'm happy to change to environment, just double checking)

https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/TargetParser/Triple.h
 only talks about "environment", which seems to be more accurate and up-to-date.

It is unfortunate that we have used our own normalization (empty string for 
unspecified environment instead of "unknown") for triple. Looks like we have to 
keep that for now. Ideally, HIP runtime should accept 
"amdgcn-amd-amdhsa-unknown" in addition to "amdgcn-amd-amdhsa--" in fat binary, 
then we could switch to the LLVM Triple normalization. But that requires 
runtime change first. Probably we could do that in future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j added a comment.

In D145770#4184996 , @yaxunl wrote:

> The description needs fix. "ABI field" should be "environment component".
>
> Also, we need a clang-offload-bundler test which bundles with a non-canonical 
> triple and unbundles with a canonical triple and vice versa.

I'm not sure if we can use Triple.normalize(). Here's what I got from a small 
test:

  amdgcn-amd-amdhsa  ->  amdgcn-amd-amdhsa (no env field added)
  amdgcn-amd-amdhsa- ->  amdgcn-amd-amdhsa-unknown (empty env changed to 
unknown)

Also are we sure ABI is incorrect? That's what is used here:

https://clang.llvm.org/docs/CrossCompilation.html

(I'm happy to change to environment, just double checking)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

The description needs fix. "ABI field" should be "environment component".

Also, we need a clang-offload-bundler test which bundles with a non-canonical 
triple and unbundles with a canonical triple and vice versa.




Comment at: clang/lib/Driver/OffloadBundler.cpp:76-79
+// Enforce optional ABI field to standardize bundles
+llvm::Triple t = llvm::Triple(KindTriple.second);
+this->Triple = llvm::Triple(t.getArchName(), t.getVendorName(),
+t.getOSName(), t.getEnvironmentName());

can we use Triple::normalize to normalize KindTriple.second, then use the 
returned string to create this->Triple? This is the conventional way to 
normalize the triple.

same as below


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j created this revision.
Herald added a subscriber: kosarev.
Herald added a project: All.
lamb-j requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

The bundler accepts both of the following for the --target option:

  hip-amdgcn-amd-amdhsa-gfx900(no ABI field)
  hip-amdgcn-amd-amdhsa--gfx900   (blank ABI field)

The ABI field is defined as optional for Triples in Triple.h.
However, in this patch we update the bundler to internally
standardize to include the ABI field. While users aren't required
to specify an ABI field when listing targets on the commandline,
bundles generated by the offload-bundler will include the ABI
field.

This standardization simplifies things for APIs that deal with
bundles generated by the clang-offload-bundler tool.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145770

Files:
  clang/lib/Driver/OffloadBundler.cpp
  clang/test/Driver/clang-offload-bundler-asserts-on.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -313,6 +313,8 @@
   llvm::DenseSet ParsedTargets;
   // Map {offload-kind}-{triple} to target IDs.
   std::map> TargetIDs;
+  // Standardize target names to include ABI field
+  std::vector StandardizedTargetNames;
   for (StringRef Target : TargetNames) {
 if (ParsedTargets.contains(Target)) {
   reportError(createStringError(errc::invalid_argument,
@@ -324,6 +326,8 @@
 bool KindIsValid = OffloadInfo.isOffloadKindValid();
 bool TripleIsValid = OffloadInfo.isTripleValid();
 
+StandardizedTargetNames.push_back(OffloadInfo.str());
+
 if (!KindIsValid || !TripleIsValid) {
   SmallVector Buf;
   raw_svector_ostream Msg(Buf);
@@ -348,6 +352,9 @@
 
 ++Index;
   }
+
+  BundlerConfig.TargetNames = StandardizedTargetNames;
+
   for (const auto  : TargetIDs) {
 if (auto ConflictingTID =
 clang::getConflictTargetIDCombination(TargetID.second)) {
Index: clang/test/Driver/clang-offload-bundler-asserts-on.c
===
--- clang/test/Driver/clang-offload-bundler-asserts-on.c
+++ clang/test/Driver/clang-offload-bundler-asserts-on.c
@@ -21,12 +21,12 @@
 
 // Tests to check compatibility between Bundle Entry ID formats i.e. between 
presence/absence of extra hyphen in case of missing environment field
 // RUN: clang-offload-bundler -unbundle -type=a 
-targets=openmp-amdgcn-amd-amdhsa--gfx906,openmp-amdgcn-amd-amdhsa-gfx908 
-input=%t.input-archive.a -output=%t-archive-gfx906-simple.a 
-output=%t-archive-gfx908-simple.a -debug-only=CodeObjectCompatibility 2>&1 | 
FileCheck %s -check-prefix=BUNDLECOMPATIBILITY
-// BUNDLECOMPATIBILITY: Compatible: Exact match:[CodeObject: 
openmp-amdgcn-amd-amdhsa-gfx906]   :   [Target: 
openmp-amdgcn-amd-amdhsa--gfx906]
-// BUNDLECOMPATIBILITY: Compatible: Exact match:[CodeObject: 
openmp-amdgcn-amd-amdhsa--gfx908]  :   [Target: 
openmp-amdgcn-amd-amdhsa-gfx908]
+// BUNDLECOMPATIBILITY: Compatible: Exact match:[CodeObject: 
openmp-amdgcn-amd-amdhsa--gfx906]  :   [Target: 
openmp-amdgcn-amd-amdhsa--gfx906]
+// BUNDLECOMPATIBILITY: Compatible: Exact match:[CodeObject: 
openmp-amdgcn-amd-amdhsa--gfx908]  :   [Target: 
openmp-amdgcn-amd-amdhsa--gfx908]
 
 // RUN: clang-offload-bundler -unbundle -type=a 
-targets=hip-amdgcn-amd-amdhsa--gfx906,hipv4-amdgcn-amd-amdhsa-gfx908 
-input=%t.input-archive.a -output=%t-hip-archive-gfx906-simple.a 
-output=%t-hipv4-archive-gfx908-simple.a -hip-openmp-compatible 
-debug-only=CodeObjectCompatibility 2>&1 | FileCheck %s 
-check-prefix=HIPOpenMPCOMPATIBILITY
-// HIPOpenMPCOMPATIBILITY: Compatible: Code Objects are compatible
[CodeObject: openmp-amdgcn-amd-amdhsa-gfx906]   :   [Target: 
hip-amdgcn-amd-amdhsa--gfx906]
-// HIPOpenMPCOMPATIBILITY: Compatible: Code Objects are compatible
[CodeObject: openmp-amdgcn-amd-amdhsa--gfx908]  :   [Target: 
hipv4-amdgcn-amd-amdhsa-gfx908]
+// HIPOpenMPCOMPATIBILITY: Compatible: Code Objects are compatible
[CodeObject: openmp-amdgcn-amd-amdhsa--gfx906]  :   [Target: 
hip-amdgcn-amd-amdhsa--gfx906]
+// HIPOpenMPCOMPATIBILITY: Compatible: Code Objects are compatible
[CodeObject: openmp-amdgcn-amd-amdhsa--gfx908]  :   [Target: 
hipv4-amdgcn-amd-amdhsa--gfx908]
 
 // Some code so that we can create a binary out of this file.
 int A = 0;
Index: clang/lib/Driver/OffloadBundler.cpp
===
--- clang/lib/Driver/OffloadBundler.cpp
+++ clang/lib/Driver/OffloadBundler.cpp
@@ -72,12 +72,22 @@
   if