From d5a7439e220c79ca9aad323f38cd115a1a34a13f Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Fri, 13 Jan 2023 13:05:36 -0500 Subject: [PATCH] [HLSL] [Dirver] add dxv as a VerifyDebug Job New option --dxv-path is added for dxc mode to set the installation path for dxv. If cannot find dxv, a warning will be report. dxv will be executed with command line dxv file_name -o file_name. It will validate and sign the file and overwrite it. Differential Revision: https://reviews.llvm.org/D141705 --- clang/include/clang/Basic/DiagnosticDriverKinds.td | 3 ++ clang/include/clang/Basic/DiagnosticGroups.td | 3 ++ clang/include/clang/Driver/Action.h | 14 +++++- clang/include/clang/Driver/Options.td | 4 ++ clang/include/clang/Driver/Types.def | 1 + clang/lib/Driver/Action.cpp | 7 +++ clang/lib/Driver/Driver.cpp | 13 +++++ clang/lib/Driver/ToolChain.cpp | 1 + clang/lib/Driver/ToolChains/HLSL.cpp | 55 +++++++++++++++++++++- clang/lib/Driver/ToolChains/HLSL.h | 24 ++++++++++ clang/test/Driver/dxc_D.hlsl | 2 +- clang/test/Driver/dxc_I.hlsl | 2 +- clang/test/Driver/dxc_dxv_path.hlsl | 23 +++++++++ clang/unittests/Driver/DXCModeTest.cpp | 4 +- 14 files changed, 150 insertions(+), 6 deletions(-) create mode 100644 clang/test/Driver/dxc_dxv_path.hlsl diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index f3d43b2..8008197 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -697,6 +697,9 @@ def err_drv_dxc_missing_target_profile : Error< "target profile option (-T) is missing">; def err_drv_hlsl_unsupported_target : Error< "HLSL code generation is unsupported for target '%0'">; +def warn_drv_dxc_missing_dxv : Warning<"dxv not found. " + "Resulting DXIL will not be validated or signed for use in release environments.">, + InGroup; def err_drv_invalid_range_dxil_validator_version : Error< "invalid validator version : %0\n" diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 6c997c3..17fdcff 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1412,6 +1412,9 @@ def BranchProtection : DiagGroup<"branch-protection">; // Warnings for HLSL Clang extensions def HLSLExtension : DiagGroup<"hlsl-extensions">; +// Warnings for DXIL validation +def DXILValidation : DiagGroup<"dxil-validation">; + // Warnings and notes related to const_var_decl_type attribute checks def ReadOnlyPlacementChecks : DiagGroup<"read-only-types">; diff --git a/clang/include/clang/Driver/Action.h b/clang/include/clang/Driver/Action.h index f8b0621..04fa8b0 100644 --- a/clang/include/clang/Driver/Action.h +++ b/clang/include/clang/Driver/Action.h @@ -75,9 +75,10 @@ public: OffloadPackagerJobClass, LinkerWrapperJobClass, StaticLibJobClass, + BinaryAnalyzeJobClass, JobClassFirst = PreprocessJobClass, - JobClassLast = StaticLibJobClass + JobClassLast = BinaryAnalyzeJobClass }; // The offloading kind determines if this action is binded to a particular @@ -674,6 +675,17 @@ public: } }; +class BinaryAnalyzeJobAction : public JobAction { + void anchor() override; + +public: + BinaryAnalyzeJobAction(Action *Input, types::ID Type); + + static bool classof(const Action *A) { + return A->getKind() == BinaryAnalyzeJobClass; + } +}; + } // namespace driver } // namespace clang diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index d8a08dc..2c7b594 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -7116,3 +7116,7 @@ def dxc_entrypoint : Option<["--", "/", "-"], "E", KIND_JOINED_OR_SEPARATE>, Group, Flags<[DXCOption, NoXarchOption]>, HelpText<"Entry point name">; +def dxc_validator_path_EQ : Joined<["--"], "dxv-path=">, Group, + HelpText<"DXIL validator installation path">; +def dxc_disable_validation : DXCFlag<"Vd">, + HelpText<"Disable validation">; diff --git a/clang/include/clang/Driver/Types.def b/clang/include/clang/Driver/Types.def index 2960f0b..aaea3ec 100644 --- a/clang/include/clang/Driver/Types.def +++ b/clang/include/clang/Driver/Types.def @@ -107,4 +107,5 @@ TYPE("dependencies", Dependencies, INVALID, "d", phases TYPE("cuda-fatbin", CUDA_FATBIN, INVALID, "fatbin", phases::Compile, phases::Backend, phases::Assemble, phases::Link) TYPE("hip-fatbin", HIP_FATBIN, INVALID, "hipfb", phases::Compile, phases::Backend, phases::Assemble, phases::Link) TYPE("api-information", API_INFO, INVALID, "json", phases::Precompile) +TYPE("dx-container", DX_CONTAINER, INVALID, "dxo", phases::Compile, phases::Backend) TYPE("none", Nothing, INVALID, nullptr, phases::Compile, phases::Backend, phases::Assemble, phases::Link) diff --git a/clang/lib/Driver/Action.cpp b/clang/lib/Driver/Action.cpp index 44b4715..849bf60 100644 --- a/clang/lib/Driver/Action.cpp +++ b/clang/lib/Driver/Action.cpp @@ -48,6 +48,8 @@ const char *Action::getClassName(ActionClass AC) { return "clang-linker-wrapper"; case StaticLibJobClass: return "static-lib-linker"; + case BinaryAnalyzeJobClass: + return "binary-analyzer"; } llvm_unreachable("invalid class"); @@ -451,3 +453,8 @@ void StaticLibJobAction::anchor() {} StaticLibJobAction::StaticLibJobAction(ActionList &Inputs, types::ID Type) : JobAction(StaticLibJobClass, Inputs, Type) {} + +void BinaryAnalyzeJobAction::anchor() {} + +BinaryAnalyzeJobAction::BinaryAnalyzeJobAction(Action *Input, types::ID Type) + : JobAction(BinaryAnalyzeJobClass, Input, Type) {} diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 661d997..98c0edc 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4212,6 +4212,18 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, I.second->claim(); } + // Call validator for dxil when -Vd not in Args. + if (C.getDefaultToolChain().getTriple().isDXIL()) { + // Only add action when needValidation. + const auto &TC = + static_cast(C.getDefaultToolChain()); + if (TC.requiresValidation(Args)) { + Action *LastAction = Actions.back(); + Actions.push_back(C.MakeAction( + LastAction, types::TY_DX_CONTAINER)); + } + } + // Claim ignored clang-cl options. Args.ClaimAllArgs(options::OPT_cl_ignored_Group); } @@ -4680,6 +4692,7 @@ void Driver::BuildJobs(Compilation &C) const { unsigned NumIfsOutputs = 0; for (const Action *A : C.getActions()) { if (A->getType() != types::TY_Nothing && + A->getType() != types::TY_DX_CONTAINER && !(A->getKind() == Action::IfsMergeJobClass || (A->getType() == clang::driver::types::TY_IFS_CPP && A->getKind() == clang::driver::Action::CompileJobClass && diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp index 61cedea..64bc97e 100644 --- a/clang/lib/Driver/ToolChain.cpp +++ b/clang/lib/Driver/ToolChain.cpp @@ -419,6 +419,7 @@ Tool *ToolChain::getTool(Action::ActionClass AC) const { case Action::LipoJobClass: case Action::DsymutilJobClass: case Action::VerifyDebugInfoJobClass: + case Action::BinaryAnalyzeJobClass: 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 1741461..c5a77e2 100644 --- a/clang/lib/Driver/ToolChains/HLSL.cpp +++ b/clang/lib/Driver/ToolChains/HLSL.cpp @@ -8,7 +8,9 @@ #include "HLSL.h" #include "CommonArgs.h" +#include "clang/Driver/Compilation.h" #include "clang/Driver/DriverDiagnostic.h" +#include "clang/Driver/Job.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/ADT/Triple.h" @@ -133,10 +135,49 @@ bool isLegalValidatorVersion(StringRef ValVersionStr, const Driver &D) { } // namespace +void tools::hlsl::Validator::ConstructJob(Compilation &C, const JobAction &JA, + const InputInfo &Output, + const InputInfoList &Inputs, + const ArgList &Args, + const char *LinkingOutput) const { + std::string DxvPath = getToolChain().GetProgramPath("dxv"); + assert(DxvPath != "dxv" && "cannot find dxv"); + + ArgStringList CmdArgs; + assert(Inputs.size() == 1 && "Unable to handle multiple inputs."); + const InputInfo &Input = Inputs[0]; + assert(Input.isFilename() && "Unexpected verify input"); + // Grabbing the output of the earlier cc1 run. + CmdArgs.push_back(Input.getFilename()); + // Use the same name as output. + CmdArgs.push_back("-o"); + CmdArgs.push_back(Input.getFilename()); + + const char *Exec = Args.MakeArgString(DxvPath); + C.addCommand(std::make_unique(JA, *this, ResponseFileSupport::None(), + Exec, CmdArgs, Inputs, Input)); +} + /// DirectX Toolchain HLSLToolChain::HLSLToolChain(const Driver &D, const llvm::Triple &Triple, const ArgList &Args) - : ToolChain(D, Triple, Args) {} + : ToolChain(D, Triple, Args) { + if (Args.hasArg(options::OPT_dxc_validator_path_EQ)) + getProgramPaths().push_back( + Args.getLastArgValue(options::OPT_dxc_validator_path_EQ).str()); +} + +Tool *clang::driver::toolchains::HLSLToolChain::getTool( + Action::ActionClass AC) const { + switch (AC) { + case Action::BinaryAnalyzeJobClass: + if (!Validator) + Validator.reset(new tools::hlsl::Validator(*this)); + return Validator.get(); + default: + return ToolChain::getTool(AC); + } +} std::optional clang::driver::toolchains::HLSLToolChain::parseTargetProfile( @@ -212,3 +253,15 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch, // See: https://github.com/llvm/llvm-project/issues/57876 return DAL; } + +bool HLSLToolChain::requiresValidation(DerivedArgList &Args) const { + if (Args.getLastArg(options::OPT_dxc_disable_validation)) + return false; + + std::string DxvPath = GetProgramPath("dxv"); + if (DxvPath != "dxv") + return true; + + getDriver().Diag(diag::warn_drv_dxc_missing_dxv); + return false; +} diff --git a/clang/lib/Driver/ToolChains/HLSL.h b/clang/lib/Driver/ToolChains/HLSL.h index 47eefdc..7b775b8 100644 --- a/clang/lib/Driver/ToolChains/HLSL.h +++ b/clang/lib/Driver/ToolChains/HLSL.h @@ -9,17 +9,37 @@ #ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_HLSL_H #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_HLSL_H +#include "clang/Driver/Tool.h" #include "clang/Driver/ToolChain.h" namespace clang { namespace driver { +namespace tools { + +namespace hlsl { +class LLVM_LIBRARY_VISIBILITY Validator : public Tool { +public: + Validator(const ToolChain &TC) : Tool("hlsl::Validator", "dxv", 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 + namespace toolchains { class LLVM_LIBRARY_VISIBILITY HLSLToolChain : public ToolChain { public: HLSLToolChain(const Driver &D, const llvm::Triple &Triple, const llvm::opt::ArgList &Args); + Tool *getTool(Action::ActionClass AC) const override; + bool isPICDefault() const override { return false; } bool isPIEDefault(const llvm::opt::ArgList &Args) const override { return false; @@ -30,6 +50,10 @@ public: TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) const override; static std::optional parseTargetProfile(StringRef TargetProfile); + bool requiresValidation(llvm::opt::DerivedArgList &Args) const; + +private: + mutable std::unique_ptr Validator; }; } // end namespace toolchains diff --git a/clang/test/Driver/dxc_D.hlsl b/clang/test/Driver/dxc_D.hlsl index d32d885..f32a22c 100644 --- a/clang/test/Driver/dxc_D.hlsl +++ b/clang/test/Driver/dxc_D.hlsl @@ -1,4 +1,4 @@ -// RUN: %clang_dxc -DTEST=2 -### %s 2>&1 | FileCheck %s +// RUN: %clang_dxc -DTEST=2 -Tlib_6_7 -### %s 2>&1 | FileCheck %s // RUN: %clang_dxc -DTEST=2 -Tlib_6_7 %s -fcgl -Fo - | FileCheck %s --check-prefix=ERROR // Make sure -D send to cc1. diff --git a/clang/test/Driver/dxc_I.hlsl b/clang/test/Driver/dxc_I.hlsl index 32491c6..166325b 100644 --- a/clang/test/Driver/dxc_I.hlsl +++ b/clang/test/Driver/dxc_I.hlsl @@ -1,4 +1,4 @@ -// RUN: %clang_dxc -I test -### %s 2>&1 | FileCheck %s +// RUN: %clang_dxc -I test -Tlib_6_3 -### %s 2>&1 | FileCheck %s // Make sure -I send to cc1. // CHECK:"-I" "test" diff --git a/clang/test/Driver/dxc_dxv_path.hlsl b/clang/test/Driver/dxc_dxv_path.hlsl new file mode 100644 index 0000000..c59c5eb --- /dev/null +++ b/clang/test/Driver/dxc_dxv_path.hlsl @@ -0,0 +1,23 @@ +// RUN: %clang_dxc -I test -Tlib_6_3 -### %s 2>&1 | FileCheck %s + +// Make sure report warning. +// CHECK:dxv not found. + +// RUN: echo "dxv" > %T/dxv && chmod 754 %T/dxv && %clang_dxc --dxv-path=%T %s -Tlib_6_3 -### 2>&1 | FileCheck %s --check-prefix=DXV_PATH +// DXV_PATH:dxv{{(.exe)?}}" "-" "-o" "-" + +// RUN: %clang_dxc -I test -Vd -Tlib_6_3 -### %s 2>&1 | FileCheck %s --check-prefix=VD +// VD:"-cc1"{{.*}}"-triple" "dxil-unknown-shadermodel6.3-library" +// VD-NOT:dxv not found + +// RUN: %clang_dxc -Tlib_6_3 -ccc-print-bindings --dxv-path=%T -Fo %t.dxo %s 2>&1 | FileCheck %s --check-prefix=BINDINGS +// BINDINGS: "dxil-unknown-shadermodel6.3-library" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[DXC:.+]].dxo" +// BINDINGS-NEXT: "dxil-unknown-shadermodel6.3-library" - "hlsl::Validator", inputs: ["[[DXC]].dxo"], output: "[[DXC]].dxo" + +// RUN: %clang_dxc -Tlib_6_3 -ccc-print-phases --dxv-path=%T -Fo %t.dxc %s 2>&1 | FileCheck %s --check-prefix=PHASES + +// PHASES: 0: input, "[[INPUT:.+]]", hlsl +// PHASES-NEXT: 1: preprocessor, {0}, c++-cpp-output +// PHASES-NEXT: 2: compiler, {1}, ir +// PHASES-NEXT: 3: backend, {2}, assembler +// PHASES-NEXT: 4: binary-analyzer, {3}, dx-container diff --git a/clang/unittests/Driver/DXCModeTest.cpp b/clang/unittests/Driver/DXCModeTest.cpp index 9ff431b..c30f3c3 100644 --- a/clang/unittests/Driver/DXCModeTest.cpp +++ b/clang/unittests/Driver/DXCModeTest.cpp @@ -34,7 +34,7 @@ static void validateTargetProfile( DiagnosticsEngine &Diags) { Driver TheDriver("/bin/clang", "", Diags, "", InMemoryFileSystem); std::unique_ptr C{TheDriver.BuildCompilation( - {"clang", "--driver-mode=dxc", TargetProfile.data(), "foo.hlsl"})}; + {"clang", "--driver-mode=dxc", TargetProfile.data(), "foo.hlsl", "-Vd"})}; EXPECT_TRUE(C); EXPECT_STREQ(TheDriver.getTargetTriple().c_str(), ExpectTriple.data()); EXPECT_EQ(Diags.getNumErrors(), 0u); @@ -47,7 +47,7 @@ static void validateTargetProfile( unsigned NumOfErrors) { Driver TheDriver("/bin/clang", "", Diags, "", InMemoryFileSystem); std::unique_ptr C{TheDriver.BuildCompilation( - {"clang", "--driver-mode=dxc", TargetProfile.data(), "foo.hlsl"})}; + {"clang", "--driver-mode=dxc", TargetProfile.data(), "foo.hlsl", "-Vd"})}; EXPECT_TRUE(C); EXPECT_EQ(Diags.getNumErrors(), NumOfErrors); EXPECT_STREQ(DiagConsumer->Errors.back().c_str(), ExpectError.data()); -- 2.7.4