[clang] d5a7439 - [HLSL] [Dirver] add dxv as a VerifyDebug Job

Xiang Li via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 1 17:22:51 PST 2023


Author: Xiang Li
Date: 2023-02-01T20:07:25-05:00
New Revision: d5a7439e220c79ca9aad323f38cd115a1a34a13f

URL: https://github.com/llvm/llvm-project/commit/d5a7439e220c79ca9aad323f38cd115a1a34a13f
DIFF: https://github.com/llvm/llvm-project/commit/d5a7439e220c79ca9aad323f38cd115a1a34a13f.diff

LOG: [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

Added: 
    clang/test/Driver/dxc_dxv_path.hlsl

Modified: 
    clang/include/clang/Basic/DiagnosticDriverKinds.td
    clang/include/clang/Basic/DiagnosticGroups.td
    clang/include/clang/Driver/Action.h
    clang/include/clang/Driver/Options.td
    clang/include/clang/Driver/Types.def
    clang/lib/Driver/Action.cpp
    clang/lib/Driver/Driver.cpp
    clang/lib/Driver/ToolChain.cpp
    clang/lib/Driver/ToolChains/HLSL.cpp
    clang/lib/Driver/ToolChains/HLSL.h
    clang/test/Driver/dxc_D.hlsl
    clang/test/Driver/dxc_I.hlsl
    clang/unittests/Driver/DXCModeTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index f3d43b2e0667..800819779890 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<DXILValidation>;
 
 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 6c997c37cc5c..17fdcffa2d42 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 f8b0621543ca..04fa8b01b418 100644
--- a/clang/include/clang/Driver/Action.h
+++ b/clang/include/clang/Driver/Action.h
@@ -75,9 +75,10 @@ class Action {
     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 @@ class StaticLibJobAction : public JobAction {
   }
 };
 
+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 d8a08dc2244e..2c7b594a60e3 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<dxc_Group>,
                      Flags<[DXCOption, NoXarchOption]>,
                      HelpText<"Entry point name">;
+def dxc_validator_path_EQ : Joined<["--"], "dxv-path=">, Group<dxc_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 2960f0ba5e9b..aaea3ec0f9c8 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 44b4715503f0..849bf6035ebd 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 661d9977fbc5..98c0edc02ba6 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<const toolchains::HLSLToolChain &>(C.getDefaultToolChain());
+    if (TC.requiresValidation(Args)) {
+      Action *LastAction = Actions.back();
+      Actions.push_back(C.MakeAction<BinaryAnalyzeJobAction>(
+          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 61cedea6cfe0..64bc97e2cb80 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 174146145777..c5a77e2bff16 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<Command>(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<std::string>
 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 47eefdc24238..7b775b897431 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 @@ class LLVM_LIBRARY_VISIBILITY HLSLToolChain : public ToolChain {
   TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,
                 Action::OffloadKind DeviceOffloadKind) const override;
   static std::optional<std::string> parseTargetProfile(StringRef TargetProfile);
+  bool requiresValidation(llvm::opt::DerivedArgList &Args) const;
+
+private:
+  mutable std::unique_ptr<tools::hlsl::Validator> Validator;
 };
 
 } // end namespace toolchains

diff  --git a/clang/test/Driver/dxc_D.hlsl b/clang/test/Driver/dxc_D.hlsl
index d32d885f11b0..f32a22c50332 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 32491c60296e..166325bdfccd 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 000000000000..c59c5ebbec75
--- /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 9ff431bb6cbb..c30f3c34a7fe 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<Compilation> 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<Compilation> 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());


        


More information about the cfe-commits mailing list