Reintroduce reverted commit "[clang-offload-bundler] Standardize TargetID field for bundler

clang/docs/ClangOffloadBundler.rst

@@ -174,7 +174,20 @@ Where:
       ============= ==============================================================
 
 **target-triple**
-    The target triple of the code object.
+    The target triple of the code object. See `Target Triple
+    <https://clang.llvm.org/docs/CrossCompilation.html#target-triple>`.
+
+    The bundler accepts target triples with or without the optional environment
+    field:
+
+    ``<arch><sub>-<vendor>-<sys>``, or
+    ``<arch><sub>-<vendor>-<sys>-<env>``
+
+    However, in order to standardize outputs for tools that consume bitcode
+    bundles, bundles written by the bundler internally use only the 4-field
+    target triple:
+
+    ``<arch><sub>-<vendor>-<sys>-<env>``
 
 **target-id**
   The canonical target ID of the code object. Present only if the target

clang/lib/Driver/Driver.cpp

@@ -115,6 +115,21 @@ using namespace clang::driver;
 using namespace clang;
 using namespace llvm::opt;
 
+// clang-offload-bundler is currently generating a 'standardized' target triple.
+// Triple's format - Architecture-Vendor-OS-Environment.
+// Bundle sections created by clang-offload-bundler contain the 'standardized'
+// triple. This routine transforms the triple specified by user as input to this
+// 'standardized' format to facilitate checks.
+static std::string standardizedTriple(std::string OrigTriple) {
+  if (OrigTriple.back() == '-') // Already standardized
+    return OrigTriple;
+  llvm::Triple t = llvm::Triple(OrigTriple);
+  return llvm::Triple(t.getArchName(), t.getVendorName(), t.getOSName(),
+                      t.getEnvironmentName())
+             .str() +
+         "-";
+}
+
 static std::optional<llvm::Triple> getOffloadTargetTriple(const Driver &D,
                                                           const ArgList &Args) {
   auto OffloadTargets = Args.getAllArgValues(options::OPT_offload_EQ);
@@ -5975,7 +5990,7 @@ class OffloadingActionBuilder final {
             Arch = C.getDriver().MakeSYCLDeviceTriple("spir64_fpga").str();
           if (std::find(UniqueSections.begin(), UniqueSections.end(), Arch) ==
               UniqueSections.end())
-            UniqueSections.push_back(Arch);
+            UniqueSections.push_back(standardizedTriple(Arch));
         }
       }
 
@@ -5988,7 +6003,7 @@ class OffloadingActionBuilder final {
           SectionTriple += "-";
           SectionTriple += SyclTarget.BoundArch;
         }
-
+        SectionTriple = standardizedTriple(SectionTriple);
         // If any matching section is found, we are good.
         if (std::find(UniqueSections.begin(), UniqueSections.end(),
                       SectionTriple) != UniqueSections.end())

clang/lib/Driver/OffloadBundler.cpp

@@ -85,12 +85,25 @@ OffloadTargetInfo::OffloadTargetInfo(const StringRef Target,
   if (clang::StringToCudaArch(TripleOrGPU.second) != clang::CudaArch::UNKNOWN) {
     auto KindTriple = TripleOrGPU.first.split('-');
     this->OffloadKind = KindTriple.first;
-    this->Triple = llvm::Triple(KindTriple.second);
-    this->TargetID = Target.substr(Target.find(TripleOrGPU.second));
+
+    // Enforce optional env field to standardize bundles
+    llvm::Triple t = llvm::Triple(KindTriple.second);
+    this->Triple = llvm::Triple(t.getArchName(), t.getVendorName(),
+                                t.getOSName(), t.getEnvironmentName());
+
+    if (TripleOrGPU.second.empty())
+      this->TargetID = "";
+    else
+      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 env 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 = "";
   }
 }
@@ -1577,34 +1590,34 @@ Error OffloadBundler::UnbundleFiles() {
   return Error::success();
 }
 
-// Unbundle the files. Return true if an error was found.
+// Unbundle the files. Return false if an error was found.
 Expected<bool>
 clang::CheckBundledSection(const OffloadBundlerConfig &BundlerConfig) {
   // Open Input file.
   ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
       MemoryBuffer::getFileOrSTDIN(BundlerConfig.InputFileNames.front());
   if (std::error_code EC = CodeOrErr.getError())
-    return createFileError(BundlerConfig.InputFileNames.front(), EC);
+    return false;
   MemoryBuffer &Input = *CodeOrErr.get();
 
   // Select the right files handler.
   Expected<std::unique_ptr<FileHandler>> FileHandlerOrErr =
       CreateFileHandler(Input, BundlerConfig);
   if (!FileHandlerOrErr)
-    return FileHandlerOrErr.takeError();
+    return false;
 
   std::unique_ptr<FileHandler> &FH = *FileHandlerOrErr;
 
   // Quit if we don't have a handler.
   if (!FH)
-    return true;
+    return false;
 
   // Seed temporary filename generation with the stem of the input file.
   FH->SetTempFileNameBase(llvm::sys::path::stem(BundlerConfig.InputFileNames.front()));
 
   // Read the header of the bundled file.
   if (Error Err = FH->ReadHeader(Input))
-    return std::move(Err);
+    return false;
 
   StringRef triple = BundlerConfig.TargetNames.front();
 
@@ -1615,13 +1628,15 @@ clang::CheckBundledSection(const OffloadBundlerConfig &BundlerConfig) {
     Expected<std::optional<StringRef>> CurTripleOrErr =
         FH->ReadBundleStart(Input);
     if (!CurTripleOrErr)
-      return CurTripleOrErr.takeError();
+      return false;
 
     // We don't have more bundles.
     if (!*CurTripleOrErr)
       break;
 
-    if (*CurTripleOrErr == triple) {
+    StringRef CurTriple = **CurTripleOrErr;
+    if (OffloadTargetInfo(CurTriple, BundlerConfig).Triple.str() ==
+        OffloadTargetInfo(triple, BundlerConfig).Triple.str()) {
       found = true;
       break;
     }

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;

clang/test/Driver/clang-offload-bundler-old-bundle-with-new-bundler.c

@@ -0,0 +1,10 @@
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: target={{.*}}-darwin{{.*}}, target={{.*}}-aix{{.*}}, system-windows
+
+// Check working of bundler before and after standardization
+// RUN: clang-offload-bundler -type=o -targets=host-x86_64-unknown-linux-gnu,sycl-spir64-unknown-unknown -input=%S/Inputs/bundles/bundle_bef_standardization_of_target_triple.o -output=test-host-x86_64-unknown-linux-gnu.o -output=test-sycl-spir64-unknown-unknown.o -unbundle 2>&1 | FileCheck %s -check-prefix=CHECK-STD-OLD --allow-empty
+// CHECK-STD-OLD-NOT: error: Can't find bundles for
+// RUN: clang-offload-bundler -type=o -targets=host-x86_64-unknown-linux-gnu,sycl-spir64-unknown-unknown -input=%S/Inputs/bundles/bundle_aft_standardization_of_target_triple.o -output=test-host-x86_64-unknown-linux-gnu.o -output=test-sycl-spir64-unknown-unknown.o -unbundle 2>&1 | FileCheck %s -check-prefix=CHECK-STD-NEW --allow-empty
+// CHECK-STD-NEW-NOT: error: Can't find bundles for
+
+

clang/test/Driver/clang-offload-bundler-standardize.c

@@ -0,0 +1,35 @@
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: target={{.*}}-darwin{{.*}}, target={{.*}}-aix{{.*}}
+
+// Generate the file we can bundle.
+// RUN: %clang -O0 -target %itanium_abi_triple %s -c -o %t.o
+
+//
+// Generate a couple of files to bundle with.
+//
+// RUN: echo 'Content of device file 1' > %t.tgt1
+// RUN: echo 'Content of device file 2' > %t.tgt2
+
+//
+// Check code object compatibility for archive unbundling
+//
+// Create an object bundle with and without env fields
+// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,hip-amdgcn-amd-amdhsa-gfx906,hip-amdgcn-amd-amdhsa-gfx908 -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle.no.env
+// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple-,hip-amdgcn-amd-amdhsa--gfx906,hip-amdgcn-amd-amdhsa--gfx908 -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bundle.env
+
+
+// Unbundle bundle.no.env while providing targets with env
+// RUN: clang-offload-bundler -unbundle -type=o -targets=hip-amdgcn-amd-amdhsa--gfx906,hip-amdgcn-amd-amdhsa--gfx908 -input=%t.bundle.no.env -output=%t-hip-amdgcn-amd-amdhsa--gfx906.bc -output=%t-hip-amdgcn-amd-amdhsa--gfx908.bc -debug-only=CodeObjectCompatibility 2>&1 | FileCheck %s -check-prefix=BUNDLE-NO-ENV
+// BUNDLE-NO-ENV: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx906] : [Target: hip-amdgcn-amd-amdhsa--gfx906]
+// BUNDLE-NO-ENV: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx908] : [Target: hip-amdgcn-amd-amdhsa--gfx908]
+
+// Unbundle bundle.env while providing targets with no env
+// RUN: clang-offload-bundler -unbundle -type=o -targets=hip-amdgcn-amd-amdhsa-gfx906,hip-amdgcn-amd-amdhsa-gfx908 -input=%t.bundle.env -output=%t-hip-amdgcn-amd-amdhsa-gfx906.bc -output=%t-hip-amdgcn-amd-amdhsa-gfx908.bc -debug-only=CodeObjectCompatibility 2>&1 | FileCheck %s -check-prefix=BUNDLE-ENV
+// BUNDLE-ENV: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx906] : [Target: hip-amdgcn-amd-amdhsa--gfx906]
+// BUNDLE-ENV: Compatible: Exact match: [CodeObject: hip-amdgcn-amd-amdhsa--gfx908] : [Target: hip-amdgcn-amd-amdhsa--gfx908]
+
+// Some code so that we can create a binary out of this file.
+int A = 0;
+void test_func(void) {
+  ++A;
+}

clang/test/Driver/clang-offload-bundler-tgtsym-asm.c

@@ -13,8 +13,8 @@
 // RUN: llvm-readobj --string-dump=.tgtsym %t.fat.o | FileCheck %s
 
 // CHECK: String dump of section '.tgtsym':
-// CHECK-DAG: openmp-x86_64-pc-linux-gnu.foo
-// CHECK-DAG: sycl-spir64.foo
+// CHECK-DAG: openmp-x86_64-pc-linux-gnu-.foo
+// CHECK-DAG: sycl-spir64----.foo
 
 __asm__(".symver memcpy,memcpy@GLIBC_2.2.5");
 void foo(void) {}

clang/test/Driver/clang-offload-bundler-tgtsym.c

@@ -12,16 +12,16 @@
 // RUN: llvm-readobj --string-dump=.tgtsym %t.fat.o | FileCheck %s
 
 // CHECK: String dump of section '.tgtsym':
-// CHECK-DAG: openmp-x86_64-pc-linux-gnu.foo
-// CHECK-DAG: openmp-x86_64-pc-linux-gnu.bar
-// CHECK-DAG: sycl-spir64.foo
-// CHECK-DAG: sycl-spir64.bar
+// CHECK-DAG: openmp-x86_64-pc-linux-gnu-.foo
+// CHECK-DAG: openmp-x86_64-pc-linux-gnu-.bar
+// CHECK-DAG: sycl-spir64----.foo
+// CHECK-DAG: sycl-spir64----.bar
 // CHECK-NOT: undefined_func
 // CHECK-NOT: static_func
 // CHECK-NOT: static_used
-// CHECK-NOT: sycl-spir64.llvm.used
-// CHECK-NOT: sycl-spir64.llvm.compiler.used
-// CHECK-NOT: sycl-spir64.const_as
+// CHECK-NOT: sycl-spir64----.llvm.used
+// CHECK-NOT: sycl-spir64----.llvm.compiler.used
+// CHECK-NOT: sycl-spir64----.const_as
 
 // RUN: clang-offload-bundler --add-target-symbols-to-bundled-object=false -type=o -targets=host-%itanium_abi_triple,openmp-x86_64-pc-linux-gnu,sycl-spir64 -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.fat.no.tgtsym.o
 // RUN: llvm-readobj --string-dump=.tgtsym %t.fat.no.tgtsym.o | FileCheck %s --check-prefix CHECK-NO-TGTSYM