jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield, tianshilei1992, tra,
yaxunl, saiislam.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay.
Herald added a project: clang.
The target features are necessary for correctly compiling most programs
in LTO mode. Currently, these are derived in clang at link time and
passed as an arguemnt to the linker wrapper. This is problematic because
it requires knowing the required toolchain at link time, which should
not be necessry. Instead, these features should be embedded into the
offloading binary so we can unify them in the linker wrapper for LTO.
This also required changing the offload packager to interpret multiple
arguments as concatenation with a comma. This is so we can still use the
`,` separator for the argument list.
Depends on D127246 <https://reviews.llvm.org/D127246>
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D127686
Files:
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/openmp-offload-gpu-new.c
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
Index: clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
===================================================================
--- clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
+++ clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
@@ -22,6 +22,7 @@
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Signals.h"
+#include "llvm/Support/StringSaver.h"
#include "llvm/Support/WithColor.h"
using namespace llvm;
@@ -71,9 +72,18 @@
SmallVector<char, 1024> BinaryData;
raw_svector_ostream OS(BinaryData);
for (StringRef Image : DeviceImages) {
+ BumpPtrAllocator Alloc;
+ StringSaver Saver(Alloc);
+
StringMap<StringRef> Args;
- for (StringRef Arg : llvm::split(Image, ","))
- Args.insert(Arg.split("="));
+ for (StringRef Arg : llvm::split(Image, ",")) {
+ auto KeyAndValue = Arg.split("=");
+ if (Args.count(KeyAndValue.first))
+ Args[KeyAndValue.first] =
+ Saver.save(Args[KeyAndValue.first] + "," + KeyAndValue.second);
+ else
+ Args[KeyAndValue.first] = KeyAndValue.second;
+ }
if (!Args.count("triple") || !Args.count("file"))
return reportError(createStringError(
Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===================================================================
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -68,10 +68,6 @@
cl::desc("Path of linker binary"),
cl::cat(ClangLinkerWrapperCategory));
-static cl::opt<std::string>
- TargetFeatures("target-feature", cl::desc("Target features for triple"),
- cl::cat(ClangLinkerWrapperCategory));
-
static cl::opt<std::string> OptLevel("opt-level",
cl::desc("Optimization level for LTO"),
cl::init("O2"),
@@ -726,16 +722,34 @@
}
}
-// Get the target features passed in from the driver as <triple>=<features>.
-std::vector<std::string> getTargetFeatures(const Triple &TheTriple) {
- std::vector<std::string> Features;
- auto TargetAndFeatures = StringRef(TargetFeatures).split('=');
- if (TargetAndFeatures.first != TheTriple.getTriple())
- return Features;
+// Get the list of target features from the input file and unify them such that
+// if there are multiple +xxx or -xxx features we only keep the last one.
+std::vector<std::string> getTargetFeatures(ArrayRef<OffloadFile> InputFiles) {
+ SmallVector<StringRef> Features;
+ for (const OffloadFile &File : InputFiles)
+ for (auto Arg : llvm::split(File.getBinary()->getString("feature"), ","))
+ Features.emplace_back(Arg);
+
+ std::vector<std::string> UnifiedFeatures;
+ llvm::StringMap<unsigned> LastOpt;
+ for (unsigned I = 0, N = Features.size(); I < N; ++I) {
+ StringRef Name = Features[I];
+ assert(Name[0] == '-' || Name[0] == '+');
+ LastOpt[Name.drop_front(1)] = I;
+ }
+
+ for (unsigned I = 0, N = Features.size(); I < N; ++I) {
+ StringRef Name = Features[I];
+ llvm::StringMap<unsigned>::iterator LastI =
+ LastOpt.find(Name.drop_front(1));
+ assert(LastI != LastOpt.end());
+ unsigned Last = LastI->second;
+ if (Last != I)
+ continue;
- for (auto Feature : llvm::split(TargetAndFeatures.second, ','))
- Features.push_back(Feature.str());
- return Features;
+ UnifiedFeatures.push_back(Name.str());
+ }
+ return UnifiedFeatures;
}
CodeGenOpt::Level getCGOptLevel(unsigned OptLevel) {
@@ -755,6 +769,7 @@
template <typename ModuleHook = function_ref<bool(size_t, const Module &)>>
std::unique_ptr<lto::LTO> createLTO(
const Triple &TheTriple, StringRef Arch, bool WholeProgram,
+ const std::vector<std::string> &Features,
ModuleHook Hook = [](size_t, const Module &) { return true; }) {
lto::Config Conf;
lto::ThinBackend Backend;
@@ -765,7 +780,7 @@
Conf.CPU = Arch.str();
Conf.Options = codegen::InitTargetOptionsFromCodeGenFlags(TheTriple);
- Conf.MAttrs = getTargetFeatures(TheTriple);
+ Conf.MAttrs = Features;
Conf.CGOptLevel = getCGOptLevel(OptLevel[1] - '0');
Conf.OptLevel = OptLevel[1] - '0';
if (Conf.OptLevel > 0)
@@ -902,10 +917,12 @@
};
// We assume visibility of the whole program if every input file was bitcode.
+ auto Features = getTargetFeatures(BitcodeInputFiles);
bool WholeProgram = InputFiles.empty();
auto LTOBackend =
- (EmbedBitcode) ? createLTO(TheTriple, Arch, WholeProgram, OutputBitcode)
- : createLTO(TheTriple, Arch, WholeProgram);
+ (EmbedBitcode)
+ ? createLTO(TheTriple, Arch, WholeProgram, Features, OutputBitcode)
+ : createLTO(TheTriple, Arch, WholeProgram, Features);
// We need to resolve the symbols so the LTO backend knows which symbols need
// to be kept or can be internalized. This is a simplified symbol resolution
Index: clang/test/Driver/openmp-offload-gpu-new.c
===================================================================
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -110,3 +110,8 @@
// RUN: %s 2>&1 | FileCheck --check-prefix=CHECK-XLINKER %s
// CHECK-XLINKER: -device-linker=a{{.*}}-device-linker=nvptx64-nvidia-cuda=b{{.*}}-device-linker=nvptx64-nvidia-cuda=c{{.*}}--
+
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fopenmp --offload-arch=sm_52 -nogpulib \
+// RUN: -foffload-lto %s 2>&1 | FileCheck --check-prefix=CHECK-LTO-FEATURES %s
+
+// CHECK-LTO-FEATURES: clang-offload-packager{{.*}}feature={{.*}}ptx
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8309,10 +8309,22 @@
? OffloadAction->getOffloadingArch()
: TCArgs.getLastArgValue(options::OPT_march_EQ);
+ ArgStringList Features;
+ SmallVector<StringRef> FeatureArgs;
+ getTargetFeatures(TC->getDriver(), TC->getTriple(), Args, Features, false);
+ llvm::copy_if(Features, std::back_inserter(FeatureArgs),
+ [](StringRef Arg) { return !Arg.startswith("-target"); });
+
+ std::string FeaturesStr =
+ TC->getDriver().isUsingLTO(/* IsOffload */ true)
+ ? ",feature=" + llvm::join(FeatureArgs, ",feature=")
+ : "";
+
CmdArgs.push_back(Args.MakeArgString(
"--image=file=" + File + "," + "triple=" + TC->getTripleString() + "," +
"arch=" + Arch + "," + "kind=" +
- Action::GetOffloadKindName(OffloadAction->getOffloadingDeviceKind())));
+ Action::GetOffloadKindName(OffloadAction->getOffloadingDeviceKind()) +
+ FeaturesStr));
}
C.addCommand(std::make_unique<Command>(
@@ -8370,20 +8382,6 @@
}
if (D.isUsingLTO(/* IsOffload */ true)) {
- // Pass in target features for each toolchain.
- for (auto &I :
- llvm::make_range(OpenMPTCRange.first, OpenMPTCRange.second)) {
- const ToolChain *TC = I.second;
- const ArgList &TCArgs = C.getArgsForToolChain(TC, "", Action::OFK_OpenMP);
- ArgStringList FeatureArgs;
- TC->addClangTargetOptions(TCArgs, FeatureArgs, Action::OFK_OpenMP);
- auto FeatureIt = llvm::find(FeatureArgs, "-target-feature");
- if (FeatureIt != FeatureArgs.end())
- CmdArgs.push_back(
- Args.MakeArgString("-target-feature=" + TC->getTripleString() +
- "=" + *(FeatureIt + 1)));
- }
-
// Pass in the optimization level to use for LTO.
if (const Arg *A = Args.getLastArg(options::OPT_O_Group)) {
StringRef OOpt;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
