[clang] [HLSL][DirectX] Add the Qdx-rootsignature-strip driver option (PR #154454)
Finn Plummer via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 22 10:41:42 PDT 2025
https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/154454
>From 0f11eb005276c35599216c60653a5126fbd7ed1e Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Fri, 8 Aug 2025 17:16:06 +0000
Subject: [PATCH 1/7] add BinaryModifyJobClass
---
clang/include/clang/Driver/Action.h | 14 +++++++++++++-
clang/lib/Driver/Action.cpp | 7 +++++++
clang/lib/Driver/ToolChain.cpp | 1 +
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Driver/Action.h b/clang/include/clang/Driver/Action.h
index 7aecfd886adb8..6fc515f9e4049 100644
--- a/clang/include/clang/Driver/Action.h
+++ b/clang/include/clang/Driver/Action.h
@@ -76,9 +76,10 @@ class Action {
StaticLibJobClass,
BinaryAnalyzeJobClass,
BinaryTranslatorJobClass,
+ BinaryModifyJobClass,
JobClassFirst = PreprocessJobClass,
- JobClassLast = BinaryTranslatorJobClass
+ JobClassLast = BinaryModifyJobClass
};
// The offloading kind determines if this action is binded to a particular
@@ -687,6 +688,17 @@ class BinaryTranslatorJobAction : public JobAction {
}
};
+class BinaryModifyJobAction : public JobAction {
+ void anchor() override;
+
+public:
+ BinaryModifyJobAction(Action *Input, types::ID Type);
+
+ static bool classof(const Action *A) {
+ return A->getKind() == BinaryModifyJobClass;
+ }
+};
+
} // namespace driver
} // namespace clang
diff --git a/clang/lib/Driver/Action.cpp b/clang/lib/Driver/Action.cpp
index ec09726044812..fbf80b3c6f124 100644
--- a/clang/lib/Driver/Action.cpp
+++ b/clang/lib/Driver/Action.cpp
@@ -52,6 +52,8 @@ const char *Action::getClassName(ActionClass AC) {
return "binary-analyzer";
case BinaryTranslatorJobClass:
return "binary-translator";
+ case BinaryModifyJobClass:
+ return "binary-modifier";
}
llvm_unreachable("invalid class");
@@ -467,3 +469,8 @@ void BinaryTranslatorJobAction::anchor() {}
BinaryTranslatorJobAction::BinaryTranslatorJobAction(Action *Input,
types::ID Type)
: JobAction(BinaryTranslatorJobClass, Input, Type) {}
+
+void BinaryModifyJobAction::anchor() {}
+
+BinaryModifyJobAction::BinaryModifyJobAction(Action *Input, types::ID Type)
+ : JobAction(BinaryModifyJobClass, Input, Type) {}
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 7667dbddb0ca2..933015ffcd0ba 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -652,6 +652,7 @@ Tool *ToolChain::getTool(Action::ActionClass AC) const {
case Action::VerifyDebugInfoJobClass:
case Action::BinaryAnalyzeJobClass:
case Action::BinaryTranslatorJobClass:
+ case Action::BinaryModifyJobClass:
llvm_unreachable("Invalid tool kind.");
case Action::CompileJobClass:
>From acfd4d525ed9c3e10f781f2c5f6a5f539bc9e7ce Mon Sep 17 00:00:00 2001
From: Finn Plummer <mail at inbelic.dev>
Date: Tue, 19 Aug 2025 18:35:51 -0700
Subject: [PATCH 2/7] [HLSL] Add the `Qstrip-rootsignature` DXC driver option
---
clang/include/clang/Basic/CodeGenOptions.def | 3 ++
clang/include/clang/Driver/Options.td | 6 +++
clang/lib/Driver/Driver.cpp | 10 +++++
clang/lib/Driver/ToolChains/HLSL.cpp | 41 ++++++++++++++++++-
clang/lib/Driver/ToolChains/HLSL.h | 15 +++++++
.../test/Driver/dxc_strip_rootsignature.hlsl | 10 +++++
6 files changed, 83 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Driver/dxc_strip_rootsignature.hlsl
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 423b696785500..89e9f5b5853bb 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -479,6 +479,9 @@ CODEGENOPT(StaticClosure, 1, 0, Benign)
/// Assume that UAVs/SRVs may alias
CODEGENOPT(ResMayAlias, 1, 0, Benign)
+/// Omit the root signature from produced DXContainer
+CODEGENOPT(HLSLRootSigStrip, 1, 0, Benign)
+
/// Controls how unwind v2 (epilog) information should be generated for x64
/// Windows.
ENUM_CODEGENOPT(WinX64EHUnwindV2, WinX64EHUnwindV2Mode,
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 6a46fec1701f3..6ca63334987ea 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -9404,6 +9404,12 @@ def res_may_alias : Option<["/", "-"], "res-may-alias", KIND_FLAG>,
Visibility<[DXCOption, ClangOption, CC1Option]>,
HelpText<"Assume that UAVs/SRVs may alias">,
MarshallingInfoFlag<CodeGenOpts<"ResMayAlias">>;
+def dxc_strip_rootsignature :
+ Option<["/", "-"], "Qstrip-rootsignature", KIND_FLAG>,
+ Group<dxc_Group>,
+ Visibility<[DXCOption]>,
+ HelpText<"Omit the root signature from produced DXContainer">,
+ MarshallingInfoFlag<CodeGenOpts<"HLSLRootSigStrip">>;
def target_profile : DXCJoinedOrSeparate<"T">, MetaVarName<"<profile>">,
HelpText<"Set target profile">,
Values<"ps_6_0, ps_6_1, ps_6_2, ps_6_3, ps_6_4, ps_6_5, ps_6_6, ps_6_7,"
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index d682ffc832c83..31b59f3759d6e 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4662,6 +4662,16 @@ void Driver::BuildDefaultActions(Compilation &C, DerivedArgList &Args,
Actions.push_back(C.MakeAction<BinaryTranslatorJobAction>(
LastAction, types::TY_DX_CONTAINER));
}
+ if (TC.requiresObjcopy(Args)) {
+ Action *LastAction = Actions.back();
+ // llvm-objcopy expects a DXIL container, which can either be
+ // validated (in which case they are TY_DX_CONTAINER), or unvalidated
+ // (TY_OBJECT).
+ if (LastAction->getType() == types::TY_DX_CONTAINER ||
+ LastAction->getType() == types::TY_Object)
+ Actions.push_back(C.MakeAction<BinaryModifyJobAction>(
+ LastAction, types::TY_DX_CONTAINER));
+ }
}
// Claim ignored clang-cl options.
diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 4fedb5dd0ac40..0853c604c09df 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -295,6 +295,30 @@ void tools::hlsl::MetalConverter::ConstructJob(
Exec, CmdArgs, Inputs, Input));
}
+void tools::hlsl::LLVMObjcopy::ConstructJob(Compilation &C, const JobAction &JA,
+ const InputInfo &Output,
+ const InputInfoList &Inputs,
+ const ArgList &Args,
+ const char *LinkingOutput) const {
+
+ std::string ObjcopyPath = getToolChain().GetProgramPath("llvm-objcopy");
+ const char *Exec = Args.MakeArgString(ObjcopyPath);
+
+ ArgStringList CmdArgs;
+ assert(Inputs.size() == 1 && "Unable to handle multiple inputs.");
+ const InputInfo &Input = Inputs[0];
+ CmdArgs.push_back(Input.getFilename());
+ CmdArgs.push_back(Output.getFilename());
+
+ if (Args.hasArg(options::OPT_dxc_strip_rootsignature)) {
+ const char *Frs = Args.MakeArgString("--remove-section=RTS0");
+ CmdArgs.push_back(Frs);
+ }
+
+ C.addCommand(std::make_unique<Command>(JA, *this, ResponseFileSupport::None(),
+ Exec, CmdArgs, Inputs, Input));
+}
+
/// DirectX Toolchain
HLSLToolChain::HLSLToolChain(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args)
@@ -315,6 +339,10 @@ Tool *clang::driver::toolchains::HLSLToolChain::getTool(
if (!MetalConverter)
MetalConverter.reset(new tools::hlsl::MetalConverter(*this));
return MetalConverter.get();
+ case Action::BinaryModifyJobClass:
+ if (!LLVMObjcopy)
+ LLVMObjcopy.reset(new tools::hlsl::LLVMObjcopy(*this));
+ return LLVMObjcopy.get();
default:
return ToolChain::getTool(AC);
}
@@ -452,16 +480,25 @@ bool HLSLToolChain::requiresBinaryTranslation(DerivedArgList &Args) const {
return Args.hasArg(options::OPT_metal) && Args.hasArg(options::OPT_dxc_Fo);
}
+bool HLSLToolChain::requiresObjcopy(DerivedArgList &Args) const {
+ return Args.hasArg(options::OPT_dxc_Fo) &&
+ Args.hasArg(options::OPT_dxc_strip_rootsignature);
+}
+
bool HLSLToolChain::isLastJob(DerivedArgList &Args,
Action::ActionClass AC) const {
bool HasTranslation = requiresBinaryTranslation(Args);
bool HasValidation = requiresValidation(Args);
+ bool HasObjcopy = requiresObjcopy(Args);
// If translation and validation are not required, we should only have one
// action.
- if (!HasTranslation && !HasValidation)
+ if (!HasTranslation && !HasValidation && !HasObjcopy)
return true;
if ((HasTranslation && AC == Action::BinaryTranslatorJobClass) ||
- (!HasTranslation && HasValidation && AC == Action::BinaryAnalyzeJobClass))
+ (!HasTranslation && HasValidation &&
+ AC == Action::BinaryAnalyzeJobClass) ||
+ (!HasTranslation && !HasValidation && HasObjcopy &&
+ AC == Action::BinaryModifyJobClass))
return true;
return false;
}
diff --git a/clang/lib/Driver/ToolChains/HLSL.h b/clang/lib/Driver/ToolChains/HLSL.h
index 3824b4252324b..b81141b26b4e6 100644
--- a/clang/lib/Driver/ToolChains/HLSL.h
+++ b/clang/lib/Driver/ToolChains/HLSL.h
@@ -42,6 +42,19 @@ class LLVM_LIBRARY_VISIBILITY MetalConverter : public Tool {
const llvm::opt::ArgList &TCArgs,
const char *LinkingOutput) const override;
};
+
+class LLVM_LIBRARY_VISIBILITY LLVMObjcopy : public Tool {
+public:
+ LLVMObjcopy(const ToolChain &TC)
+ : Tool("hlsl::LLVMObjcopy", "llvm-objcopy", TC) {}
+
+ bool hasIntegratedCPP() const override { return false; }
+
+ void ConstructJob(Compilation &C, const JobAction &JA,
+ const InputInfo &Output, const InputInfoList &Inputs,
+ const llvm::opt::ArgList &TCArgs,
+ const char *LinkingOutput) const override;
+};
} // namespace hlsl
} // namespace tools
@@ -65,6 +78,7 @@ class LLVM_LIBRARY_VISIBILITY HLSLToolChain : public ToolChain {
static std::optional<std::string> parseTargetProfile(StringRef TargetProfile);
bool requiresValidation(llvm::opt::DerivedArgList &Args) const;
bool requiresBinaryTranslation(llvm::opt::DerivedArgList &Args) const;
+ bool requiresObjcopy(llvm::opt::DerivedArgList &Args) const;
bool isLastJob(llvm::opt::DerivedArgList &Args, Action::ActionClass AC) const;
// Set default DWARF version to 4 for DXIL uses version 4.
@@ -73,6 +87,7 @@ class LLVM_LIBRARY_VISIBILITY HLSLToolChain : public ToolChain {
private:
mutable std::unique_ptr<tools::hlsl::Validator> Validator;
mutable std::unique_ptr<tools::hlsl::MetalConverter> MetalConverter;
+ mutable std::unique_ptr<tools::hlsl::LLVMObjcopy> LLVMObjcopy;
};
} // end namespace toolchains
diff --git a/clang/test/Driver/dxc_strip_rootsignature.hlsl b/clang/test/Driver/dxc_strip_rootsignature.hlsl
new file mode 100644
index 0000000000000..23bb39ded8a1b
--- /dev/null
+++ b/clang/test/Driver/dxc_strip_rootsignature.hlsl
@@ -0,0 +1,10 @@
+// RUN: %clang_dxc -Qstrip-rootsignature -T cs_6_0 /Fo %t -### %s 2>&1 | FileCheck %s
+
+// Test to demonstrate that we specify to the root signature with the
+// -Qstrip-rootsignature option
+
+// CHECK: "{{.*}}llvm-objcopy{{.*}}" "{{.*}}" "{{.*}}" "--remove-section=RTS0"
+
+[shader("compute"), RootSignature("")]
+[numthreads(1,1,1)]
+void EmptyEntry() {}
>From f3f3a0728a5071fa9f566e83a15a2c0e7caac09f Mon Sep 17 00:00:00 2001
From: Finn Plummer <mail at inbelic.dev>
Date: Thu, 21 Aug 2025 11:20:51 -0700
Subject: [PATCH 3/7] review: use terminology directed for objcopy
---
clang/include/clang/Driver/Action.h | 10 +++++-----
clang/lib/Driver/Action.cpp | 10 +++++-----
clang/lib/Driver/Driver.cpp | 4 ++--
clang/lib/Driver/ToolChain.cpp | 2 +-
clang/lib/Driver/ToolChains/HLSL.cpp | 16 ++++++++--------
clang/lib/Driver/ToolChains/HLSL.h | 9 +++++----
6 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/clang/include/clang/Driver/Action.h b/clang/include/clang/Driver/Action.h
index 6fc515f9e4049..dbf1187da4db9 100644
--- a/clang/include/clang/Driver/Action.h
+++ b/clang/include/clang/Driver/Action.h
@@ -76,10 +76,10 @@ class Action {
StaticLibJobClass,
BinaryAnalyzeJobClass,
BinaryTranslatorJobClass,
- BinaryModifyJobClass,
+ ObjcopyJobClass,
JobClassFirst = PreprocessJobClass,
- JobClassLast = BinaryModifyJobClass
+ JobClassLast = ObjcopyJobClass
};
// The offloading kind determines if this action is binded to a particular
@@ -688,14 +688,14 @@ class BinaryTranslatorJobAction : public JobAction {
}
};
-class BinaryModifyJobAction : public JobAction {
+class ObjcopyJobAction : public JobAction {
void anchor() override;
public:
- BinaryModifyJobAction(Action *Input, types::ID Type);
+ ObjcopyJobAction(Action *Input, types::ID Type);
static bool classof(const Action *A) {
- return A->getKind() == BinaryModifyJobClass;
+ return A->getKind() == ObjcopyJobClass;
}
};
diff --git a/clang/lib/Driver/Action.cpp b/clang/lib/Driver/Action.cpp
index fbf80b3c6f124..e19daa9cb7abf 100644
--- a/clang/lib/Driver/Action.cpp
+++ b/clang/lib/Driver/Action.cpp
@@ -52,8 +52,8 @@ const char *Action::getClassName(ActionClass AC) {
return "binary-analyzer";
case BinaryTranslatorJobClass:
return "binary-translator";
- case BinaryModifyJobClass:
- return "binary-modifier";
+ case ObjcopyJobClass:
+ return "objcopy";
}
llvm_unreachable("invalid class");
@@ -470,7 +470,7 @@ BinaryTranslatorJobAction::BinaryTranslatorJobAction(Action *Input,
types::ID Type)
: JobAction(BinaryTranslatorJobClass, Input, Type) {}
-void BinaryModifyJobAction::anchor() {}
+void ObjcopyJobAction::anchor() {}
-BinaryModifyJobAction::BinaryModifyJobAction(Action *Input, types::ID Type)
- : JobAction(BinaryModifyJobClass, Input, Type) {}
+ObjcopyJobAction::ObjcopyJobAction(Action *Input, types::ID Type)
+ : JobAction(ObjcopyJobClass, Input, Type) {}
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 31b59f3759d6e..ea7a1b5dd4041 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4669,8 +4669,8 @@ void Driver::BuildDefaultActions(Compilation &C, DerivedArgList &Args,
// (TY_OBJECT).
if (LastAction->getType() == types::TY_DX_CONTAINER ||
LastAction->getType() == types::TY_Object)
- Actions.push_back(C.MakeAction<BinaryModifyJobAction>(
- LastAction, types::TY_DX_CONTAINER));
+ Actions.push_back(
+ C.MakeAction<ObjcopyJobAction>(LastAction, types::TY_DX_CONTAINER));
}
}
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 933015ffcd0ba..f645220f30bfd 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -652,7 +652,7 @@ Tool *ToolChain::getTool(Action::ActionClass AC) const {
case Action::VerifyDebugInfoJobClass:
case Action::BinaryAnalyzeJobClass:
case Action::BinaryTranslatorJobClass:
- case Action::BinaryModifyJobClass:
+ case Action::ObjcopyJobClass:
llvm_unreachable("Invalid tool kind.");
case Action::CompileJobClass:
diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 0853c604c09df..ad21cf55ffec8 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -295,11 +295,11 @@ void tools::hlsl::MetalConverter::ConstructJob(
Exec, CmdArgs, Inputs, Input));
}
-void tools::hlsl::LLVMObjcopy::ConstructJob(Compilation &C, const JobAction &JA,
- const InputInfo &Output,
- const InputInfoList &Inputs,
- const ArgList &Args,
- const char *LinkingOutput) const {
+void tools::LLVMObjcopy::ConstructJob(Compilation &C, const JobAction &JA,
+ const InputInfo &Output,
+ const InputInfoList &Inputs,
+ const ArgList &Args,
+ const char *LinkingOutput) const {
std::string ObjcopyPath = getToolChain().GetProgramPath("llvm-objcopy");
const char *Exec = Args.MakeArgString(ObjcopyPath);
@@ -339,9 +339,9 @@ Tool *clang::driver::toolchains::HLSLToolChain::getTool(
if (!MetalConverter)
MetalConverter.reset(new tools::hlsl::MetalConverter(*this));
return MetalConverter.get();
- case Action::BinaryModifyJobClass:
+ case Action::ObjcopyJobClass:
if (!LLVMObjcopy)
- LLVMObjcopy.reset(new tools::hlsl::LLVMObjcopy(*this));
+ LLVMObjcopy.reset(new tools::LLVMObjcopy(*this));
return LLVMObjcopy.get();
default:
return ToolChain::getTool(AC);
@@ -498,7 +498,7 @@ bool HLSLToolChain::isLastJob(DerivedArgList &Args,
(!HasTranslation && HasValidation &&
AC == Action::BinaryAnalyzeJobClass) ||
(!HasTranslation && !HasValidation && HasObjcopy &&
- AC == Action::BinaryModifyJobClass))
+ AC == Action::ObjcopyJobClass))
return true;
return false;
}
diff --git a/clang/lib/Driver/ToolChains/HLSL.h b/clang/lib/Driver/ToolChains/HLSL.h
index b81141b26b4e6..36b155baa8f0c 100644
--- a/clang/lib/Driver/ToolChains/HLSL.h
+++ b/clang/lib/Driver/ToolChains/HLSL.h
@@ -43,10 +43,11 @@ class LLVM_LIBRARY_VISIBILITY MetalConverter : public Tool {
const char *LinkingOutput) const override;
};
+} // namespace hlsl
+
class LLVM_LIBRARY_VISIBILITY LLVMObjcopy : public Tool {
public:
- LLVMObjcopy(const ToolChain &TC)
- : Tool("hlsl::LLVMObjcopy", "llvm-objcopy", TC) {}
+ LLVMObjcopy(const ToolChain &TC) : Tool("LLVMObjcopy", "llvm-objcopy", TC) {}
bool hasIntegratedCPP() const override { return false; }
@@ -55,7 +56,7 @@ class LLVM_LIBRARY_VISIBILITY LLVMObjcopy : public Tool {
const llvm::opt::ArgList &TCArgs,
const char *LinkingOutput) const override;
};
-} // namespace hlsl
+
} // namespace tools
namespace toolchains {
@@ -87,7 +88,7 @@ class LLVM_LIBRARY_VISIBILITY HLSLToolChain : public ToolChain {
private:
mutable std::unique_ptr<tools::hlsl::Validator> Validator;
mutable std::unique_ptr<tools::hlsl::MetalConverter> MetalConverter;
- mutable std::unique_ptr<tools::hlsl::LLVMObjcopy> LLVMObjcopy;
+ mutable std::unique_ptr<tools::LLVMObjcopy> LLVMObjcopy;
};
} // end namespace toolchains
>From f8ff016249b312d2e1397b10f62d4abbaa371c7f Mon Sep 17 00:00:00 2001
From: Finn Plummer <mail at inbelic.dev>
Date: Thu, 21 Aug 2025 11:50:19 -0700
Subject: [PATCH 4/7] review: add assert
---
clang/lib/Driver/ToolChains/HLSL.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index ad21cf55ffec8..7274928225fe2 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -315,6 +315,8 @@ void tools::LLVMObjcopy::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back(Frs);
}
+ assert(CmdArgs.size() > 2 && "Unnecessary invocation of objcopy.");
+
C.addCommand(std::make_unique<Command>(JA, *this, ResponseFileSupport::None(),
Exec, CmdArgs, Inputs, Input));
}
>From 154c2b0a11132031aa38473c6105636d9de5cb58 Mon Sep 17 00:00:00 2001
From: Finn Plummer <mail at inbelic.dev>
Date: Thu, 21 Aug 2025 11:51:43 -0700
Subject: [PATCH 5/7] review: apply objcopy before validation
- DXC will invoke the validator after the final blob has been generated
(in this case after the part is removed)
---
clang/lib/Driver/Driver.cpp | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index ea7a1b5dd4041..b266749c4d38c 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4647,6 +4647,16 @@ void Driver::BuildDefaultActions(Compilation &C, DerivedArgList &Args,
// Only add action when needValidation.
const auto &TC =
static_cast<const toolchains::HLSLToolChain &>(C.getDefaultToolChain());
+ if (TC.requiresObjcopy(Args)) {
+ Action *LastAction = Actions.back();
+ // llvm-objcopy expects a DXIL container, which can either be
+ // validated (in which case they are TY_DX_CONTAINER), or unvalidated
+ // (TY_OBJECT).
+ if (LastAction->getType() == types::TY_DX_CONTAINER ||
+ LastAction->getType() == types::TY_Object)
+ Actions.push_back(
+ C.MakeAction<ObjcopyJobAction>(LastAction, types::TY_DX_CONTAINER));
+ }
if (TC.requiresValidation(Args)) {
Action *LastAction = Actions.back();
Actions.push_back(C.MakeAction<BinaryAnalyzeJobAction>(
@@ -4662,16 +4672,6 @@ void Driver::BuildDefaultActions(Compilation &C, DerivedArgList &Args,
Actions.push_back(C.MakeAction<BinaryTranslatorJobAction>(
LastAction, types::TY_DX_CONTAINER));
}
- if (TC.requiresObjcopy(Args)) {
- Action *LastAction = Actions.back();
- // llvm-objcopy expects a DXIL container, which can either be
- // validated (in which case they are TY_DX_CONTAINER), or unvalidated
- // (TY_OBJECT).
- if (LastAction->getType() == types::TY_DX_CONTAINER ||
- LastAction->getType() == types::TY_Object)
- Actions.push_back(
- C.MakeAction<ObjcopyJobAction>(LastAction, types::TY_DX_CONTAINER));
- }
}
// Claim ignored clang-cl options.
>From a0ff7f79584a6050df8c2ae47c9e4f458c9cb847 Mon Sep 17 00:00:00 2001
From: Finn Plummer <mail at inbelic.dev>
Date: Fri, 22 Aug 2025 10:35:00 -0700
Subject: [PATCH 6/7] review: improve comments
---
clang/lib/Driver/Driver.cpp | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index b266749c4d38c..f1c81cf0b196b 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4642,26 +4642,28 @@ void Driver::BuildDefaultActions(Compilation &C, DerivedArgList &Args,
}
}
- // Call validator for dxil when -Vd not in Args.
if (C.getDefaultToolChain().getTriple().isDXIL()) {
- // Only add action when needValidation.
const auto &TC =
static_cast<const toolchains::HLSLToolChain &>(C.getDefaultToolChain());
+
+ // Call objcopy for manipulation of the DXContainer when an option in Args
+ // requires it.
if (TC.requiresObjcopy(Args)) {
Action *LastAction = Actions.back();
- // llvm-objcopy expects a DXIL container, which can either be
- // validated (in which case they are TY_DX_CONTAINER), or unvalidated
- // (TY_OBJECT).
- if (LastAction->getType() == types::TY_DX_CONTAINER ||
- LastAction->getType() == types::TY_Object)
+ // llvm-objcopy expects an unvalidated DXIL container (TY_OBJECT).
+ if (LastAction->getType() == types::TY_Object)
Actions.push_back(
C.MakeAction<ObjcopyJobAction>(LastAction, types::TY_DX_CONTAINER));
}
+
+ // Call validator for dxil when -Vd not in Args.
if (TC.requiresValidation(Args)) {
Action *LastAction = Actions.back();
Actions.push_back(C.MakeAction<BinaryAnalyzeJobAction>(
LastAction, types::TY_DX_CONTAINER));
}
+
+ // Call metal-shaderconverter when targeting metal.
if (TC.requiresBinaryTranslation(Args)) {
Action *LastAction = Actions.back();
// Metal shader converter runs on DXIL containers, which can either be
>From 2b6f936cd567a917b8d1a8a3e3a684a08b594382 Mon Sep 17 00:00:00 2001
From: Finn Plummer <mail at inbelic.dev>
Date: Fri, 22 Aug 2025 10:38:33 -0700
Subject: [PATCH 7/7] re-review: make this llvmobcopy hlsl specific
- following convention used from other toolchains (eg. `Linker`), each
toolchain will need to define a custom override of `ConstructJob` so
that it converts the applicable arguments accordingly
- since this will be toolchain specific, it is convention to prefix it
with the toolchain namespace (hlsl::)
---
clang/lib/Driver/ToolChains/HLSL.cpp | 12 ++++++------
clang/lib/Driver/ToolChains/HLSL.h | 8 ++++----
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 7274928225fe2..b0164189dcf92 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -295,11 +295,11 @@ void tools::hlsl::MetalConverter::ConstructJob(
Exec, CmdArgs, Inputs, Input));
}
-void tools::LLVMObjcopy::ConstructJob(Compilation &C, const JobAction &JA,
- const InputInfo &Output,
- const InputInfoList &Inputs,
- const ArgList &Args,
- const char *LinkingOutput) const {
+void tools::hlsl::LLVMObjcopy::ConstructJob(Compilation &C, const JobAction &JA,
+ const InputInfo &Output,
+ const InputInfoList &Inputs,
+ const ArgList &Args,
+ const char *LinkingOutput) const {
std::string ObjcopyPath = getToolChain().GetProgramPath("llvm-objcopy");
const char *Exec = Args.MakeArgString(ObjcopyPath);
@@ -343,7 +343,7 @@ Tool *clang::driver::toolchains::HLSLToolChain::getTool(
return MetalConverter.get();
case Action::ObjcopyJobClass:
if (!LLVMObjcopy)
- LLVMObjcopy.reset(new tools::LLVMObjcopy(*this));
+ LLVMObjcopy.reset(new tools::hlsl::LLVMObjcopy(*this));
return LLVMObjcopy.get();
default:
return ToolChain::getTool(AC);
diff --git a/clang/lib/Driver/ToolChains/HLSL.h b/clang/lib/Driver/ToolChains/HLSL.h
index 36b155baa8f0c..eb976cbe501e7 100644
--- a/clang/lib/Driver/ToolChains/HLSL.h
+++ b/clang/lib/Driver/ToolChains/HLSL.h
@@ -43,11 +43,10 @@ class LLVM_LIBRARY_VISIBILITY MetalConverter : public Tool {
const char *LinkingOutput) const override;
};
-} // namespace hlsl
-
class LLVM_LIBRARY_VISIBILITY LLVMObjcopy : public Tool {
public:
- LLVMObjcopy(const ToolChain &TC) : Tool("LLVMObjcopy", "llvm-objcopy", TC) {}
+ LLVMObjcopy(const ToolChain &TC)
+ : Tool("hlsl::LLVMObjcopy", "llvm-objcopy", TC) {}
bool hasIntegratedCPP() const override { return false; }
@@ -57,6 +56,7 @@ class LLVM_LIBRARY_VISIBILITY LLVMObjcopy : public Tool {
const char *LinkingOutput) const override;
};
+} // namespace hlsl
} // namespace tools
namespace toolchains {
@@ -88,7 +88,7 @@ class LLVM_LIBRARY_VISIBILITY HLSLToolChain : public ToolChain {
private:
mutable std::unique_ptr<tools::hlsl::Validator> Validator;
mutable std::unique_ptr<tools::hlsl::MetalConverter> MetalConverter;
- mutable std::unique_ptr<tools::LLVMObjcopy> LLVMObjcopy;
+ mutable std::unique_ptr<tools::hlsl::LLVMObjcopy> LLVMObjcopy;
};
} // end namespace toolchains
More information about the cfe-commits
mailing list