[clang] 1172bde - [HIP] Fix unbundling archive
Yaxun Liu via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 26 19:19:50 PDT 2022
Author: Yaxun (Sam) Liu
Date: 2022-09-26T22:16:17-04:00
New Revision: 1172bdecfab364579d90e6aa5ba7fc64a5b96786
URL: https://github.com/llvm/llvm-project/commit/1172bdecfab364579d90e6aa5ba7fc64a5b96786
DIFF: https://github.com/llvm/llvm-project/commit/1172bdecfab364579d90e6aa5ba7fc64a5b96786.diff
LOG: [HIP] Fix unbundling archive
HIP is able to unbundle archive of bundled bitcode.
However currently there are two bugs:
1. archives passed by -l: are not unbundled.
2. archives passed as input files are not unbundled
The actual file name of an archive passed by -l: should
not be prefixed with lib and appended with '.a',
but the file path is prefixed with paths in '-L' options.
The actual file name of an archive passed as an input file
stays the same, not affected by the '-L' options.
Added:
Modified:
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/test/Driver/clang-offload-bundler.c
clang/test/Driver/hip-link-bundle-archive.hip
Removed:
################################################################################
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 85c4bb46019fa..c16797f318f6d 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -2909,12 +2909,16 @@ class OffloadingActionBuilder final {
std::string FileName = IA->getInputArg().getAsString(Args);
// Check if the type of the file is the same as the action. Do not
// unbundle it if it is not. Do not unbundle .so files, for example,
- // which are not object files.
+ // which are not object files. Files with extension ".lib" is classified
+ // as TY_Object but they are actually archives, therefore should not be
+ // unbundled here as objects. They will be handled at other places.
+ const StringRef LibFileExt = ".lib";
if (IA->getType() == types::TY_Object &&
(!llvm::sys::path::has_extension(FileName) ||
types::lookupTypeForExtension(
llvm::sys::path::extension(FileName).drop_front()) !=
- types::TY_Object))
+ types::TY_Object ||
+ llvm::sys::path::extension(FileName) == LibFileExt))
return ABRT_Inactive;
for (auto Arch : GpuArchList) {
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 22025d95e7c8f..86ed6edfb86fa 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -40,6 +40,7 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/Twine.h"
+#include "llvm/BinaryFormat/Magic.h"
#include "llvm/Config/llvm-config.h"
#include "llvm/Option/Arg.h"
#include "llvm/Option/ArgList.h"
@@ -1812,85 +1813,99 @@ bool tools::GetSDLFromOffloadArchive(
return false;
bool FoundAOB = false;
- SmallVector<std::string, 2> AOBFileNames;
std::string ArchiveOfBundles;
- for (auto LPath : LibraryPaths) {
- ArchiveOfBundles.clear();
-
- llvm::Triple Triple(D.getTargetTriple());
- bool IsMSVC = Triple.isWindowsMSVCEnvironment();
- for (auto Prefix : {"/libdevice/", "/"}) {
- if (IsMSVC)
- AOBFileNames.push_back(Twine(LPath + Prefix + Lib + ".lib").str());
- AOBFileNames.push_back(Twine(LPath + Prefix + "lib" + Lib + ".a").str());
- }
- for (auto AOB : AOBFileNames) {
- if (llvm::sys::fs::exists(AOB)) {
- ArchiveOfBundles = AOB;
- FoundAOB = true;
- break;
+ llvm::Triple Triple(D.getTargetTriple());
+ bool IsMSVC = Triple.isWindowsMSVCEnvironment();
+ auto Ext = IsMSVC ? ".lib" : ".a";
+ if (!Lib.startswith(":") && llvm::sys::fs::exists(Lib)) {
+ ArchiveOfBundles = Lib;
+ FoundAOB = true;
+ } else {
+ for (auto LPath : LibraryPaths) {
+ ArchiveOfBundles.clear();
+ SmallVector<std::string, 2> AOBFileNames;
+ auto LibFile =
+ (Lib.startswith(":") ? Lib.drop_front()
+ : IsMSVC ? Lib + Ext : "lib" + Lib + Ext)
+ .str();
+ for (auto Prefix : {"/libdevice/", "/"}) {
+ auto AOB = Twine(LPath + Prefix + LibFile).str();
+ if (llvm::sys::fs::exists(AOB)) {
+ ArchiveOfBundles = AOB;
+ FoundAOB = true;
+ break;
+ }
}
+ if (FoundAOB)
+ break;
}
+ }
- if (!FoundAOB)
- continue;
-
- StringRef Prefix = isBitCodeSDL ? "libbc-" : "lib";
- std::string OutputLib = D.GetTemporaryPath(
- Twine(Prefix + Lib + "-" + Arch + "-" + Target).str(), "a");
+ if (!FoundAOB)
+ return false;
- C.addTempFile(C.getArgs().MakeArgString(OutputLib));
+ llvm::file_magic Magic;
+ auto EC = llvm::identify_magic(ArchiveOfBundles, Magic);
+ if (EC || Magic != llvm::file_magic::archive)
+ return false;
- ArgStringList CmdArgs;
- SmallString<128> DeviceTriple;
- DeviceTriple += Action::GetOffloadKindName(JA.getOffloadingDeviceKind());
+ StringRef Prefix = isBitCodeSDL ? "libbc-" : "lib";
+ std::string OutputLib =
+ D.GetTemporaryPath(Twine(Prefix + llvm::sys::path::filename(Lib) + "-" +
+ Arch + "-" + Target)
+ .str(),
+ "a");
+
+ C.addTempFile(C.getArgs().MakeArgString(OutputLib));
+
+ ArgStringList CmdArgs;
+ SmallString<128> DeviceTriple;
+ DeviceTriple += Action::GetOffloadKindName(JA.getOffloadingDeviceKind());
+ DeviceTriple += '-';
+ std::string NormalizedTriple = T.getToolChain().getTriple().normalize();
+ DeviceTriple += NormalizedTriple;
+ if (!Target.empty()) {
DeviceTriple += '-';
- std::string NormalizedTriple = T.getToolChain().getTriple().normalize();
- DeviceTriple += NormalizedTriple;
- if (!Target.empty()) {
- DeviceTriple += '-';
- DeviceTriple += Target;
- }
+ DeviceTriple += Target;
+ }
- std::string UnbundleArg("-unbundle");
- std::string TypeArg("-type=a");
- std::string InputArg("-input=" + ArchiveOfBundles);
- std::string OffloadArg("-targets=" + std::string(DeviceTriple));
- std::string OutputArg("-output=" + OutputLib);
-
- const char *UBProgram = DriverArgs.MakeArgString(
- T.getToolChain().GetProgramPath("clang-offload-bundler"));
-
- ArgStringList UBArgs;
- UBArgs.push_back(C.getArgs().MakeArgString(UnbundleArg));
- UBArgs.push_back(C.getArgs().MakeArgString(TypeArg));
- UBArgs.push_back(C.getArgs().MakeArgString(InputArg));
- UBArgs.push_back(C.getArgs().MakeArgString(OffloadArg));
- UBArgs.push_back(C.getArgs().MakeArgString(OutputArg));
-
- // Add this flag to not exit from clang-offload-bundler if no compatible
- // code object is found in heterogenous archive library.
- std::string AdditionalArgs("-allow-missing-bundles");
- UBArgs.push_back(C.getArgs().MakeArgString(AdditionalArgs));
-
- // Add this flag to treat hip and hipv4 offload kinds as compatible with
- // openmp offload kind while extracting code objects from a heterogenous
- // archive library. Vice versa is also considered compatible.
- std::string HipCompatibleArgs("-hip-openmp-compatible");
- UBArgs.push_back(C.getArgs().MakeArgString(HipCompatibleArgs));
-
- C.addCommand(std::make_unique<Command>(
- JA, T, ResponseFileSupport::AtFileCurCP(), UBProgram, UBArgs, Inputs,
- InputInfo(&JA, C.getArgs().MakeArgString(OutputLib))));
- if (postClangLink)
- CC1Args.push_back("-mlink-builtin-bitcode");
+ std::string UnbundleArg("-unbundle");
+ std::string TypeArg("-type=a");
+ std::string InputArg("-input=" + ArchiveOfBundles);
+ std::string OffloadArg("-targets=" + std::string(DeviceTriple));
+ std::string OutputArg("-output=" + OutputLib);
- CC1Args.push_back(DriverArgs.MakeArgString(OutputLib));
- break;
- }
+ const char *UBProgram = DriverArgs.MakeArgString(
+ T.getToolChain().GetProgramPath("clang-offload-bundler"));
+
+ ArgStringList UBArgs;
+ UBArgs.push_back(C.getArgs().MakeArgString(UnbundleArg));
+ UBArgs.push_back(C.getArgs().MakeArgString(TypeArg));
+ UBArgs.push_back(C.getArgs().MakeArgString(InputArg));
+ UBArgs.push_back(C.getArgs().MakeArgString(OffloadArg));
+ UBArgs.push_back(C.getArgs().MakeArgString(OutputArg));
+
+ // Add this flag to not exit from clang-offload-bundler if no compatible
+ // code object is found in heterogenous archive library.
+ std::string AdditionalArgs("-allow-missing-bundles");
+ UBArgs.push_back(C.getArgs().MakeArgString(AdditionalArgs));
+
+ // Add this flag to treat hip and hipv4 offload kinds as compatible with
+ // openmp offload kind while extracting code objects from a heterogenous
+ // archive library. Vice versa is also considered compatible.
+ std::string HipCompatibleArgs("-hip-openmp-compatible");
+ UBArgs.push_back(C.getArgs().MakeArgString(HipCompatibleArgs));
- return FoundAOB;
+ C.addCommand(std::make_unique<Command>(
+ JA, T, ResponseFileSupport::AtFileCurCP(), UBProgram, UBArgs, Inputs,
+ InputInfo(&JA, C.getArgs().MakeArgString(OutputLib))));
+ if (postClangLink)
+ CC1Args.push_back("-mlink-builtin-bitcode");
+
+ CC1Args.push_back(DriverArgs.MakeArgString(OutputLib));
+
+ return true;
}
// Wrapper function used by driver for adding SDLs during link phase.
@@ -1980,6 +1995,22 @@ void tools::AddStaticDeviceLibs(Compilation *C, const Tool *T,
}
}
+ for (auto Input : DriverArgs.getAllArgValues(options::OPT_INPUT)) {
+ auto FileName = StringRef(Input);
+ // Clang treats any unknown file types as archives and passes them to the
+ // linker. Files with extension 'lib' are classified as TY_Object by clang
+ // but they are usually archives. It is OK if the file is not really an
+ // archive since GetSDLFromOffloadArchive will check the magic of the file
+ // and only unbundle it if it is really an archive.
+ const StringRef LibFileExt = ".lib";
+ if (!llvm::sys::path::has_extension(FileName) ||
+ types::lookupTypeForExtension(
+ llvm::sys::path::extension(FileName).drop_front()) ==
+ types::TY_INVALID ||
+ llvm::sys::path::extension(FileName) == LibFileExt)
+ SDLNames.insert(Input);
+ }
+
// The search stops as soon as an SDL file is found. The driver then provides
// the full filename of the SDL to the llvm-link or clang-nvlink-wrapper
// command. If no SDL is found after searching each LINKPATH with
diff --git a/clang/test/Driver/clang-offload-bundler.c b/clang/test/Driver/clang-offload-bundler.c
index 555029bd838ff..965c619dd14a6 100644
--- a/clang/test/Driver/clang-offload-bundler.c
+++ b/clang/test/Driver/clang-offload-bundler.c
@@ -407,6 +407,17 @@
// HIP-AR-906-DAG: hip_bundle1-hip-amdgcn-amd-amdhsa--gfx906
// HIP-AR-906-DAG: hip_bundle2-hip-amdgcn-amd-amdhsa--gfx906
+// Check clang-offload-bundler reporting an error when trying to unbundle an archive but
+// the input file is not an archive.
+//
+// RUN: echo 'This is not an archive file.' > %t.non-archive
+// RUN: not clang-offload-bundler -unbundle -type=a -targets=hip-amdgcn-amd-amdhsa--gfx900 \
+// RUN: -output=%t.res.a -input=%t.non-archive --allow-missing-bundles 2>&1 \
+// RUN: | FileCheck %s -check-prefix=INVARCHIVE
+// RUN: not clang-offload-bundler -unbundle -type=a -targets=hip-amdgcn-amd-amdhsa--gfx900 \
+// RUN: -output=%t.res.a -input=%t.non-archive 2>&1 | FileCheck %s -check-prefix=INVARCHIVE
+// INVARCHIVE: error: file too small to be an archive
+
//
// Check bundling without host target is allowed for HIP.
//
diff --git a/clang/test/Driver/hip-link-bundle-archive.hip b/clang/test/Driver/hip-link-bundle-archive.hip
index 32cec5fab9def..5f5017d998b0d 100644
--- a/clang/test/Driver/hip-link-bundle-archive.hip
+++ b/clang/test/Driver/hip-link-bundle-archive.hip
@@ -1,29 +1,77 @@
// REQUIRES: x86-registered-target, amdgpu-registered-target
-
// Check clang unbundle the archive and link them by lld.
// RUN: rm -rf %t && mkdir %t
-// RUN: touch %t/libhipBundled.a
+// RUN: touch %t/dummy.bc
+// RUN: llvm-ar cr %t/libhipBundled.a %t/dummy.bc
// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
// RUN: --target=x86_64-unknown-linux-gnu \
// RUN: -nogpulib %s -fgpu-rdc -L%t -lhipBundled \
-// RUN: 2>&1 | FileCheck -check-prefix=GNU %s
+// RUN: 2>&1 | FileCheck -check-prefixes=GNU,GNU1,GNU-L %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN: --target=x86_64-unknown-linux-gnu \
+// RUN: -nogpulib %s -fgpu-rdc -L%t -l:libhipBundled.a \
+// RUN: 2>&1 | FileCheck -check-prefixes=GNU,GNU1,GNU-LA %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN: --target=x86_64-unknown-linux-gnu \
+// RUN: -nogpulib %s -fgpu-rdc %t/libhipBundled.a \
+// RUN: 2>&1 | FileCheck -check-prefixes=GNU,GNU1,GNU-A %s
-// RUN: touch %t/hipBundled2.lib
+// RUN: llvm-ar cr %t/libhipBundled.a.5.2 %t/dummy.bc
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN: --target=x86_64-unknown-linux-gnu \
+// RUN: -nogpulib %s -fgpu-rdc %t/libhipBundled.a.5.2 \
+// RUN: 2>&1 | FileCheck -check-prefixes=GNU,GNU2,GNU-A %s
+
+// Check if a file is not an archive, it is not unbundled.
+
+// RUN: touch %t/libNonArchive.a
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN: --target=x86_64-unknown-linux-gnu \
+// RUN: -nogpulib %s -fgpu-rdc -L%t -lNonArchive \
+// RUN: 2>&1 | FileCheck -check-prefixes=NONARCHIVE %s
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN: --target=x86_64-unknown-linux-gnu \
+// RUN: -nogpulib %s -fgpu-rdc -L%t -l:libNonArchive.a \
+// RUN: 2>&1 | FileCheck -check-prefixes=NONARCHIVE %s
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN: --target=x86_64-unknown-linux-gnu \
+// RUN: -nogpulib %s -fgpu-rdc -L%t libNonArchive.a \
+// RUN: 2>&1 | FileCheck -check-prefixes=NONARCHIVE %s
+
+// Check unbundling archive for MSVC.
+
+// RUN: llvm-ar cr %t/hipBundled2.lib %t/dummy.bc
// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
// RUN: --target=x86_64-pc-windows-msvc \
// RUN: -nogpulib %s -fgpu-rdc -L%t -lhipBundled2 \
// RUN: 2>&1 | FileCheck -check-prefix=MSVC %s
-// GNU: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libhipBundled.a" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN: --target=x86_64-pc-windows-msvc \
+// RUN: -nogpulib %s -fgpu-rdc -L%t -l:hipBundled2.lib \
+// RUN: 2>&1 | FileCheck -check-prefix=MSVC %s
+
+// RUN: %clang -### --offload-arch=gfx906 --offload-arch=gfx1030 \
+// RUN: --target=x86_64-pc-windows-msvc \
+// RUN: -nogpulib %s -fgpu-rdc %t/hipBundled2.lib \
+// RUN: 2>&1 | FileCheck -check-prefix=MSVC %s
+
+// GNU1: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}[[LIB:libhipBundled\.a]]" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
+// GNU2: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}[[LIB:libhipBundled\.a\.5\.2]]" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
// GNU: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx1030" {{.*}} "[[A1030]]"
-// GNU: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}libhipBundled.a" "-targets=hip-amdgcn-amd-amdhsa-gfx906" "-output=[[A906:.*\.a]]" "-allow-missing-bundles"
+// GNU: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}[[LIB]]" "-targets=hip-amdgcn-amd-amdhsa-gfx906" "-output=[[A906:.*\.a]]" "-allow-missing-bundles"
// GNU: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx906" {{.*}} "[[A906]]"
-// GNU: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-lhipBundled"
+// GNU-L: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-lhipBundled"
+// GNU-LA: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" {{.*}}"-l:libhipBundled.a"
+// GNU-A: "{{.*}}ld{{.*}}" {{.*}}"-o" "a.out" "{{.*}}[[LIB]]"
+// NONARCHIVE-NOT: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*libNonArchive\.a}}"
// MSVC: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}hipBundled2.lib" "-targets=hip-amdgcn-amd-amdhsa-gfx1030" "-output=[[A1030:.*\.a]]" "-allow-missing-bundles"
// MSVC: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx1030" {{.*}} "[[A1030]]"
// MSVC: "{{.*}}clang-offload-bundler" "-unbundle" "-type=a" "-input={{.*}}hipBundled2.lib" "-targets=hip-amdgcn-amd-amdhsa-gfx906" "-output=[[A906:.*\.a]]" "-allow-missing-bundles"
// MSVC: "{{.*}}lld{{.*}}" {{.*}}"-plugin-opt=mcpu=gfx906" {{.*}} "[[A906]]"
-// MSVC: "{{.*}}link{{.*}}" {{.*}}"-out:a.exe" {{.*}}"hipBundled2.lib"
+// MSVC: "{{.*}}link{{.*}}" {{.*}}"-out:a.exe" {{.*}}hipBundled2.lib"
More information about the cfe-commits
mailing list