[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<StringRef> ParsedTargets;
   // Map {offload-kind}-{triple} to target IDs.
   std::map<std::string, std::set<StringRef>> TargetIDs;
+  // Standardize target names to include ABI field
+  std::vector<std::string> 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<char, 128u> Buf;
       raw_svector_ostream Msg(Buf);
@@ -348,6 +352,9 @@
 
     ++Index;
   }
+
+  BundlerConfig.TargetNames = StandardizedTargetNames;
+
   for (const auto &TargetID : 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 (clang::StringToCudaArch(TripleOrGPU.second) != clang::CudaArch::UNKNOWN) {
     auto KindTriple = TripleOrGPU.first.split('-');
     this->OffloadKind = KindTriple.first;
-    this->Triple = llvm::Triple(KindTriple.second);
+
+    // 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());
+
     this->TargetID = Target.substr(Target.find(TripleOrGPU.second));
   } else {
     auto KindTriple = TargetFeatures.first.split('-');
     this->OffloadKind = KindTriple.first;
-    this->Triple = llvm::Triple(KindTriple.second);
+
+    // 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());
+
     this->TargetID = "";
   }
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D145770.504050.patch
Type: text/x-patch
Size: 4916 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230310/e42bdbb6/attachment.bin>