https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/209721
>From 437a679908a92000fff1ed05d1c0d8c615c28f4e Mon Sep 17 00:00:00 2001 From: Matt Arsenault <[email protected]> Date: Wed, 15 Jul 2026 11:53:26 +0200 Subject: [PATCH] clang-offload-bundler: Remove conflicting-target-ID diagnostic This was checking that you did not try to combine arches with any mode vs. a specific mode (e.g, gfx90a and gfx90a:xnack+). I don't see any point in this diagnostic. All the modes have a natural interpretation as distinct targets with a selection preference. It's more defensible to have this rule in the user facing clang driver, but not the low level binary utilities. This reduces the surface area of some special case target ID parsing. Co-authored-by: Claude (Opus 4.8) --- clang/docs/ClangOffloadBundler.rst | 6 -- clang/include/clang/Driver/OffloadBundler.h | 1 - clang/lib/Driver/OffloadBundler.cpp | 78 ------------------- .../clang-offload-bundler/basic.c | 21 +++-- .../ClangOffloadBundler.cpp | 43 +--------- 5 files changed, 18 insertions(+), 131 deletions(-) diff --git a/clang/docs/ClangOffloadBundler.rst b/clang/docs/ClangOffloadBundler.rst index f63037a742088..6e49878d04dfc 100644 --- a/clang/docs/ClangOffloadBundler.rst +++ b/clang/docs/ClangOffloadBundler.rst @@ -56,7 +56,6 @@ objects appropriate for the devices present when the host program is executed. --### - Print any external commands that are to be executed instead of actually executing them - for testing purposes. --allow-missing-bundles - Create empty files if bundles are missing when unbundling. --bundle-align=<uint> - Alignment of bundle for binary files - --check-input-archive - Check if input heterogeneous archive is valid in terms of TargetID rules. --inputs=<string> - [<input file>,...] --list - List bundle IDs in the bundled file. --outputs=<string> - [<output file>,...] @@ -502,11 +501,6 @@ Additional Options while Archive Unbundling **-allow-missing-bundles** Create an empty archive file if no compatible device binary is found. -**-check-input-archive** - Check if input heterogeneous device archive follows rules for composition - as defined in :ref:`code-object-composition` before creating device-specific - archive(s). - **-debug-only=CodeObjectCompatibility** Verbose printing of matched/unmatched comparisons between bundle entry id of a device binary from HDA and bundle entry ID of a given target processor diff --git a/clang/include/clang/Driver/OffloadBundler.h b/clang/include/clang/Driver/OffloadBundler.h index 96839240543af..ffb874cf562cc 100644 --- a/clang/include/clang/Driver/OffloadBundler.h +++ b/clang/include/clang/Driver/OffloadBundler.h @@ -32,7 +32,6 @@ class OffloadBundlerConfig { bool AllowNoHost = false; bool AllowMissingBundles = false; - bool CheckInputArchive = false; bool PrintExternalCommands = false; bool HipOpenmpCompatible = false; bool Compress = false; diff --git a/clang/lib/Driver/OffloadBundler.cpp b/clang/lib/Driver/OffloadBundler.cpp index 8e4d44071ef55..23dc1b8cbef35 100644 --- a/clang/lib/Driver/OffloadBundler.cpp +++ b/clang/lib/Driver/OffloadBundler.cpp @@ -1483,74 +1483,6 @@ getCompatibleOffloadTargets(OffloadTargetInfo &CodeObjectInfo, return !CompatibleTargets.empty(); } -// Check that each code object file in the input archive conforms to following -// rule: for a specific processor, a feature either shows up in all target IDs, -// or does not show up in any target IDs. Otherwise the target ID combination is -// invalid. -static Error -CheckHeterogeneousArchive(StringRef ArchiveName, - const OffloadBundlerConfig &BundlerConfig) { - std::vector<std::unique_ptr<MemoryBuffer>> ArchiveBuffers; - ErrorOr<std::unique_ptr<MemoryBuffer>> BufOrErr = - MemoryBuffer::getFileOrSTDIN(ArchiveName, true, false); - if (std::error_code EC = BufOrErr.getError()) - return createFileError(ArchiveName, EC); - - ArchiveBuffers.push_back(std::move(*BufOrErr)); - Expected<std::unique_ptr<llvm::object::Archive>> LibOrErr = - Archive::create(ArchiveBuffers.back()->getMemBufferRef()); - if (!LibOrErr) - return LibOrErr.takeError(); - - auto Archive = std::move(*LibOrErr); - - Error ArchiveErr = Error::success(); - auto ChildEnd = Archive->child_end(); - - /// Iterate over all bundled code object files in the input archive. - for (auto ArchiveIter = Archive->child_begin(ArchiveErr); - ArchiveIter != ChildEnd; ++ArchiveIter) { - if (ArchiveErr) - return ArchiveErr; - auto ArchiveChildNameOrErr = (*ArchiveIter).getName(); - if (!ArchiveChildNameOrErr) - return ArchiveChildNameOrErr.takeError(); - - auto CodeObjectBufferRefOrErr = (*ArchiveIter).getMemoryBufferRef(); - if (!CodeObjectBufferRefOrErr) - return CodeObjectBufferRefOrErr.takeError(); - - auto CodeObjectBuffer = - MemoryBuffer::getMemBuffer(*CodeObjectBufferRefOrErr, false); - - Expected<std::unique_ptr<FileHandler>> FileHandlerOrErr = - CreateFileHandler(*CodeObjectBuffer, BundlerConfig); - if (!FileHandlerOrErr) - return FileHandlerOrErr.takeError(); - - std::unique_ptr<FileHandler> &FileHandler = *FileHandlerOrErr; - assert(FileHandler); - - std::set<StringRef> BundleIds; - auto CodeObjectFileError = - FileHandler->getBundleIDs(*CodeObjectBuffer, BundleIds); - if (CodeObjectFileError) - return CodeObjectFileError; - - auto &&ConflictingArchs = clang::getConflictTargetIDCombination(BundleIds); - if (ConflictingArchs) { - std::string ErrMsg = - Twine("conflicting TargetIDs [" + ConflictingArchs.value().first + - ", " + ConflictingArchs.value().second + "] found in " + - ArchiveChildNameOrErr.get() + " of " + ArchiveName) - .str(); - return createStringError(inconvertibleErrorCode(), ErrMsg); - } - } - - return ArchiveErr; -} - /// UnbundleArchive takes an archive file (".a") as input containing bundled /// code object files, and a list of offload targets (not host), and extracts /// the code objects into a new archive file for each offload target. Each @@ -1576,16 +1508,6 @@ Error OffloadBundler::UnbundleArchive() { StringRef IFName = BundlerConfig.InputFileNames.front(); - if (BundlerConfig.CheckInputArchive) { - // For a specific processor, a feature either shows up in all target IDs, or - // does not show up in any target IDs. Otherwise the target ID combination - // is invalid. - auto ArchiveError = CheckHeterogeneousArchive(IFName, BundlerConfig); - if (ArchiveError) { - return ArchiveError; - } - } - ErrorOr<std::unique_ptr<MemoryBuffer>> BufOrErr = MemoryBuffer::getFileOrSTDIN(IFName, true, false); if (std::error_code EC = BufOrErr.getError()) diff --git a/clang/test/OffloadTools/clang-offload-bundler/basic.c b/clang/test/OffloadTools/clang-offload-bundler/basic.c index 21ac93cc66245..8043918652696 100644 --- a/clang/test/OffloadTools/clang-offload-bundler/basic.c +++ b/clang/test/OffloadTools/clang-offload-bundler/basic.c @@ -478,11 +478,16 @@ // RUN: clang-offload-bundler -type=bc -targets=hip-amdgcn-amd-amdhsa--gfx906:xnack- \ // RUN: -targets=hip-amdgcn-amd-amdhsa--gfx906:xnack+ \ // RUN: -input=%t.tgt1 -input=%t.tgt2 -output=%t.hip.bundle.bc -// RUN: not clang-offload-bundler -type=bc -targets=hip-amdgcn-amd-amdhsa--gfx906 \ +// +// A processor with a feature specified in one target and unspecified in another +// is not treated as a conflict; the inputs are simply bundled together. +// RUN: clang-offload-bundler -type=bc -targets=hip-amdgcn-amd-amdhsa--gfx906 \ // RUN: -targets=hip-amdgcn-amd-amdhsa--gfx906:xnack+ \ -// RUN: -input=%t.tgt1 -input=%t.tgt2 -output=%t.hip.bundle.bc 2>&1 \ -// RUN: | FileCheck %s -check-prefix=CONFLICT-TID -// CONFLICT-TID: error: Cannot bundle inputs with conflicting targets: 'hip-amdgcn-amd-amdhsa--gfx906' and 'hip-amdgcn-amd-amdhsa--gfx906:xnack+' +// RUN: -input=%t.tgt1 -input=%t.tgt2 -output=%t.hip.mixed.bundle.bc +// RUN: clang-offload-bundler -type=bc -input=%t.hip.mixed.bundle.bc -list \ +// RUN: | FileCheck %s -check-prefix=MIXED-TID +// MIXED-TID-DAG: hip-amdgcn-amd-amdhsa--gfx906{{$}} +// MIXED-TID-DAG: hip-amdgcn-amd-amdhsa--gfx906:xnack+ // // Check extracting bundle entry with compatible target ID for HIP. @@ -532,8 +537,12 @@ // GFX906: simple-openmp-amdgcn-amd-amdhsa--gfx906 // RUN: llvm-ar t %t-archive-gfx908-simple.a | FileCheck %s -check-prefix=GFX908 // GFX908-NOT: {{gfx906}} -// RUN: not clang-offload-bundler -type=o -targets=host-x86_64-unknown-linux-gnu,openmp-amdgcn-amd-amdhsa--gfx906,openmp-amdgcn-amd-amdhsa--gfx906:sramecc+ -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bad.bundle 2>&1 | FileCheck %s -check-prefix=BADTARGETS -// BADTARGETS: error: Cannot bundle inputs with conflicting targets: 'openmp-amdgcn-amd-amdhsa--gfx906' and 'openmp-amdgcn-amd-amdhsa--gfx906:sramecc+' +// A processor with a feature specified in one target and unspecified in another +// is not a conflict; the inputs are bundled together. +// RUN: clang-offload-bundler -type=o -targets=host-x86_64-unknown-linux-gnu,openmp-amdgcn-amd-amdhsa--gfx906,openmp-amdgcn-amd-amdhsa--gfx906:sramecc+ -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.mixed.bundle +// RUN: clang-offload-bundler -type=o -input=%t.mixed.bundle -list | FileCheck %s -check-prefix=MIXED-SRAMECC +// MIXED-SRAMECC-DAG: openmp-amdgcn-amd-amdhsa--gfx906{{$}} +// MIXED-SRAMECC-DAG: openmp-amdgcn-amd-amdhsa--gfx906:sramecc+ // Check for error if no compatible code object is found in the heterogeneous archive library // RUN: not clang-offload-bundler -unbundle -type=a -targets=openmp-amdgcn-amd-amdhsa--gfx803 -input=%t.input-archive.a -output=%t-archive-gfx803-incompatible.a 2>&1 | FileCheck %s -check-prefix=INCOMPATIBLEARCHIVE diff --git a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp index 40d77abe2ef7c..70532a918e812 100644 --- a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp +++ b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp @@ -13,7 +13,6 @@ //===----------------------------------------------------------------------===// #include "clang/Basic/Cuda.h" -#include "clang/Basic/TargetID.h" #include "clang/Basic/Version.h" #include "clang/Driver/OffloadBundler.h" #include "llvm/ADT/ArrayRef.h" @@ -126,15 +125,9 @@ int main(int argc, const char **argv) { cl::desc("Create empty files if bundles are missing " "when unbundling.\n"), cl::init(false), cl::cat(ClangOffloadBundlerCategory)); - cl::opt<unsigned> - BundleAlignment("bundle-align", - cl::desc("Alignment of bundle for binary files"), - cl::init(1), cl::cat(ClangOffloadBundlerCategory)); - cl::opt<bool> CheckInputArchive( - "check-input-archive", - cl::desc("Check if input heterogeneous archive is " - "valid in terms of TargetID rules.\n"), - cl::init(false), cl::cat(ClangOffloadBundlerCategory)); + cl::opt<unsigned> BundleAlignment( + "bundle-align", cl::desc("Alignment of bundle for binary files"), + cl::init(1), cl::cat(ClangOffloadBundlerCategory)); cl::opt<bool> HipOpenmpCompatible( "hip-openmp-compatible", cl::desc("Treat hip and hipv4 offload kinds as " @@ -170,7 +163,6 @@ int main(int argc, const char **argv) { // Avoid using cl::opt variables after these assignments when possible OffloadBundlerConfig BundlerConfig; BundlerConfig.AllowMissingBundles = AllowMissingBundles; - BundlerConfig.CheckInputArchive = CheckInputArchive; BundlerConfig.PrintExternalCommands = PrintExternalCommands; BundlerConfig.HipOpenmpCompatible = HipOpenmpCompatible; BundlerConfig.BundleAlignment = BundleAlignment; @@ -290,19 +282,6 @@ int main(int argc, const char **argv) { }); } - if (BundlerConfig.CheckInputArchive) { - if (!Unbundle) { - return reportError(createStringError( - errc::invalid_argument, "-check-input-archive cannot be used while " - "bundling")); - } - if (Unbundle && BundlerConfig.FilesType != "a") { - return reportError(createStringError( - errc::invalid_argument, "-check-input-archive can only be used for " - "unbundling archives (-type=a)")); - } - } - if (OutputFileNames.size() == 0) { return reportError( createStringError(errc::invalid_argument, "no output file specified!")); @@ -349,8 +328,6 @@ int main(int argc, const char **argv) { unsigned HostTargetNum = 0u; bool HIPOnly = true; llvm::DenseSet<StringRef> ParsedTargets; - // Map {offload-kind}-{triple} to target IDs. - std::map<std::string, std::set<StringRef>> TargetIDs; // Standardize target names to include env field std::vector<std::string> StandardizedTargetNames; for (StringRef Target : TargetNames) { @@ -385,8 +362,6 @@ int main(int argc, const char **argv) { return reportError(createStringError(errc::invalid_argument, Msg.str())); } - TargetIDs[OffloadInfo.OffloadKind.str() + "-" + OffloadInfo.Triple.str()] - .insert(OffloadInfo.TargetID); if (KindIsValid && OffloadInfo.hasHostKind()) { ++HostTargetNum; // Save the index of the input that refers to the host. @@ -402,18 +377,6 @@ int main(int argc, const char **argv) { BundlerConfig.TargetNames.assign(StandardizedTargetNames.begin(), StandardizedTargetNames.end()); - for (const auto &TargetID : TargetIDs) { - if (auto ConflictingTID = - clang::getConflictTargetIDCombination(TargetID.second)) { - SmallVector<char, 128u> Buf; - raw_svector_ostream Msg(Buf); - Msg << "Cannot bundle inputs with conflicting targets: '" - << TargetID.first + "-" + ConflictingTID->first << "' and '" - << TargetID.first + "-" + ConflictingTID->second << "'"; - return reportError(createStringError(errc::invalid_argument, Msg.str())); - } - } - // HIP uses clang-offload-bundler to bundle device-only compilation results // for multiple GPU archs, therefore allow no host target if all entries // are for HIP. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
