Skip to content
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/CodeGenOptions.def
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this anymore? I don't see it being used, and I would think that making this a separate tool's job would make this unnecessary.


/// Controls how unwind v2 (epilog) information should be generated for x64
/// Windows.
ENUM_CODEGENOPT(WinX64EHUnwindV2, WinX64EHUnwindV2Mode,
Expand Down
14 changes: 13 additions & 1 deletion clang/include/clang/Driver/Action.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,10 @@ class Action {
StaticLibJobClass,
BinaryAnalyzeJobClass,
BinaryTranslatorJobClass,
ObjcopyJobClass,

JobClassFirst = PreprocessJobClass,
JobClassLast = BinaryTranslatorJobClass
JobClassLast = ObjcopyJobClass
};

// The offloading kind determines if this action is binded to a particular
Expand Down Expand Up @@ -687,6 +688,17 @@ class BinaryTranslatorJobAction : public JobAction {
}
};

class ObjcopyJobAction : public JobAction {
void anchor() override;

public:
ObjcopyJobAction(Action *Input, types::ID Type);

static bool classof(const Action *A) {
return A->getKind() == ObjcopyJobClass;
}
};

} // namespace driver
} // namespace clang

Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -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,"
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/Driver/Action.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ const char *Action::getClassName(ActionClass AC) {
return "binary-analyzer";
case BinaryTranslatorJobClass:
return "binary-translator";
case ObjcopyJobClass:
return "objcopy";
}

llvm_unreachable("invalid class");
Expand Down Expand Up @@ -467,3 +469,8 @@ void BinaryTranslatorJobAction::anchor() {}
BinaryTranslatorJobAction::BinaryTranslatorJobAction(Action *Input,
types::ID Type)
: JobAction(BinaryTranslatorJobClass, Input, Type) {}

void ObjcopyJobAction::anchor() {}

ObjcopyJobAction::ObjcopyJobAction(Action *Input, types::ID Type)
: JobAction(ObjcopyJobClass, Input, Type) {}
10 changes: 10 additions & 0 deletions clang/lib/Driver/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4647,6 +4647,16 @@ void Driver::BuildDefaultActions(Compilation &C, DerivedArgList &Args,
// Only add action when needValidation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments around the isDXIL block look like they're out of date or at least misplaced (both this one and the "Call validator" one above.

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 ||
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be running this before validation rather than after? What does DXC do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, DXC will construct the final output blob before running the validator. So it would validate with the parts removed.

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>(
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Driver/ToolChain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ Tool *ToolChain::getTool(Action::ActionClass AC) const {
case Action::VerifyDebugInfoJobClass:
case Action::BinaryAnalyzeJobClass:
case Action::BinaryTranslatorJobClass:
case Action::ObjcopyJobClass:
llvm_unreachable("Invalid tool kind.");

case Action::CompileJobClass:
Expand Down
43 changes: 41 additions & 2 deletions clang/lib/Driver/ToolChains/HLSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,32 @@ 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 {

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);
}

assert(CmdArgs.size() > 2 && "Unnecessary invocation of objcopy.");

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)
Expand All @@ -315,6 +341,10 @@ Tool *clang::driver::toolchains::HLSLToolChain::getTool(
if (!MetalConverter)
MetalConverter.reset(new tools::hlsl::MetalConverter(*this));
return MetalConverter.get();
case Action::ObjcopyJobClass:
if (!LLVMObjcopy)
LLVMObjcopy.reset(new tools::LLVMObjcopy(*this));
return LLVMObjcopy.get();
default:
return ToolChain::getTool(AC);
}
Expand Down Expand Up @@ -452,16 +482,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::ObjcopyJobClass))
return true;
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is getting a bit unweildy. Would it simplify things to work from the last job backwards? That is,

if (HasObjcopy)
  return AC == Action::ObjcopyJobClass;
if (HasValidation)
  return AC == Action::BinaryAnalyzeJobClass;
if (HasTranslation)
  return AC == Action::BinaryTranslatorJobClass;
// If none of translation, validation, or objcopy are required, we should only
// have one action
return true;

}
16 changes: 16 additions & 0 deletions clang/lib/Driver/ToolChains/HLSL.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,21 @@ class LLVM_LIBRARY_VISIBILITY MetalConverter : public Tool {
const llvm::opt::ArgList &TCArgs,
const char *LinkingOutput) const override;
};

} // namespace hlsl

class LLVM_LIBRARY_VISIBILITY LLVMObjcopy : public Tool {
public:
LLVMObjcopy(const ToolChain &TC) : Tool("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;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LLVMObjcopy fairly generic, even if we are only using it in the HLSL toolchain right now. Should this class be defined somewhere less toolchain specific?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it is generic, the logic of ConstructJob is specific to just the hlsl toolchain.

It is convention for the other toolchains to each redefine their own version of the struct with their namespace prefixed. See Linker etc.

So I have added back the hlsl namespace to this struct.


} // namespace tools

namespace toolchains {
Expand All @@ -65,6 +79,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.
Expand All @@ -73,6 +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;
};

} // end namespace toolchains
Expand Down
10 changes: 10 additions & 0 deletions clang/test/Driver/dxc_strip_rootsignature.hlsl
Original file line number Diff line number Diff line change
@@ -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() {}
Loading