https://github.com/bob80905 updated https://github.com/llvm/llvm-project/pull/188150
>From 26229c0a9b848bbbd02e42b644b465f4c4ae224b Mon Sep 17 00:00:00 2001 From: Joshua Batista <[email protected]> Date: Mon, 23 Mar 2026 16:31:43 -0700 Subject: [PATCH 1/5] first attempt --- .../clang/Basic/DiagnosticDriverKinds.td | 3 + clang/include/clang/Basic/DiagnosticGroups.td | 3 + clang/include/clang/Options/Options.td | 2 + clang/lib/Driver/Driver.cpp | 38 +++++++++--- clang/lib/Driver/ToolChains/HLSL.cpp | 58 ++++++++++++++++--- clang/lib/Driver/ToolChains/HLSL.h | 17 +++++- clang/test/Driver/dxc_spirv-val_path.hlsl | 26 +++++++++ 7 files changed, 131 insertions(+), 16 deletions(-) create mode 100644 clang/test/Driver/dxc_spirv-val_path.hlsl diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index ed6a9107002af..89486026d9b48 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -873,6 +873,9 @@ def err_drv_hlsl_bad_shader_unsupported : Error< def warn_drv_dxc_missing_dxv : Warning< "dxv not found; resulting DXIL will not be validated or signed for use in " "release environment">, InGroup<DXILValidation>; +def warn_drv_dxc_missing_spirv_val : Warning< + "spirv-val not found; resulting SPIR-V will not be validated">, + InGroup<SPIRVValidation>; def err_drv_invalid_range_dxil_validator_version : Error< "invalid validator version : %0; validator version must be less than or " diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index e440c9d2fb982..11fe963f8066a 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1875,6 +1875,9 @@ def HLSLExplicitBinding : DiagGroup<"hlsl-explicit-binding">; // Warnings for DXIL validation def DXILValidation : DiagGroup<"dxil-validation">; +// Warnings for SPIRV validation +def SPIRVValidation : DiagGroup<"spirv-validation">; + // Warning for HLSL API availability def HLSLAvailability : DiagGroup<"hlsl-availability">; diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index 8b0c701521728..9b7909b1a5d60 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -9976,6 +9976,8 @@ def dxc_hlsl_version : Option<["/", "-"], "HV", KIND_JOINED_OR_SEPARATE>, Values<"2016, 2017, 2018, 2021, 202x, 202y">; def dxc_validator_path_EQ : Joined<["--"], "dxv-path=">, Group<dxc_Group>, HelpText<"DXIL validator installation path">; +def spirv_validator_path_EQ : Joined<["--"], "spirv-val-path=">, Group<dxc_Group>, + HelpText<"SPIRV validator installation path">; def dxc_disable_validation : DXCFlag<"Vd">, HelpText<"Disable validation">; def dxc_gis : DXCFlag<"Gis">, diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 3e2553686d87f..70dd8285f346a 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4739,6 +4739,18 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, } } + if (C.getDefaultToolChain().getTriple().isSPIRV()) { + const auto &TC = + static_cast<const toolchains::HLSLToolChain &>(C.getDefaultToolChain()); + + // Call spirv-val for SPIR-V when -Vd not in Args. + if (TC.requiresValidation(Args)) { + Action *LastAction = Actions.back(); + Actions.push_back( + C.MakeAction<BinaryAnalyzeJobAction>(LastAction, types::TY_Object)); + } + } + // Claim ignored clang-cl options. Args.ClaimAllArgs(options::OPT_cl_ignored_Group); } @@ -5381,11 +5393,9 @@ void Driver::BuildJobs(Compilation &C) const { unsigned NumOutputs = 0; unsigned NumIfsOutputs = 0; for (const Action *A : C.getActions()) { - // The actions below do not increase the number of outputs, when operating - // on DX containers. - if (A->getType() == types::TY_DX_CONTAINER && - (A->getKind() == clang::driver::Action::BinaryAnalyzeJobClass || - A->getKind() == clang::driver::Action::BinaryTranslatorJobClass)) + // The actions below do not increase the number of outputs. + if (A->getKind() == clang::driver::Action::BinaryAnalyzeJobClass || + A->getKind() == clang::driver::Action::BinaryTranslatorJobClass) continue; if (A->getType() != types::TY_Nothing && @@ -6434,10 +6444,22 @@ const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA, const char *Suffix = types::getTypeTempSuffix(JA.getType(), true); return CreateTempFile(C, Split.first, Suffix, false); } - // We don't have SPIRV-val integrated (yet), so for now we can assume this - // is the final output. + assert(C.getDefaultToolChain().getTriple().isSPIRV()); - return C.addResultFile(C.getArgs().MakeArgString(FoValue.str()), &JA); + const auto &TC = + static_cast<const toolchains::HLSLToolChain &>(C.getDefaultToolChain()); + + // If this is the last job in the compilation for this input, and Fo is + // non-empty, we can return the Fo file (the final output) + if (TC.isLastJob(C.getArgs(), JA.getKind()) && !FoValue.empty()) + return C.addResultFile(C.getArgs().MakeArgString(FoValue.str()), &JA); + + // Otherwise, create a temporary file for validation (like DXIL creates + // temp files). + StringRef Name = llvm::sys::path::filename(BaseInput); + std::pair<StringRef, StringRef> Split = Name.split('.'); + const char *Suffix = types::getTypeTempSuffix(JA.getType(), true); + return CreateTempFile(C, Split.first, Suffix, false); } // Is this the assembly listing for /FA? diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp index 1070f0ef6ee9e..be0e2ccdc911b 100644 --- a/clang/lib/Driver/ToolChains/HLSL.cpp +++ b/clang/lib/Driver/ToolChains/HLSL.cpp @@ -315,6 +315,23 @@ void tools::hlsl::Validator::ConstructJob(Compilation &C, const JobAction &JA, Exec, CmdArgs, Inputs, Input)); } +void tools::hlsl::SPIRV_Validator::ConstructJob( + Compilation &C, const JobAction &JA, const InputInfo &Output, + const InputInfoList &Inputs, const ArgList &Args, + const char *LinkingOutput) const { + std::string SPIRVValPath = getToolChain().GetProgramPath("spirv-val"); + assert(SPIRVValPath != "spirv-val" && "cannot find spirv-val"); + + ArgStringList CmdArgs; + assert(Inputs.size() == 1 && "Unable to handle multiple inputs."); + const InputInfo &Input = Inputs[0]; + CmdArgs.push_back(Input.getFilename()); + + const char *Exec = Args.MakeArgString(SPIRVValPath); + C.addCommand(std::make_unique<Command>(JA, *this, ResponseFileSupport::None(), + Exec, CmdArgs, Inputs, Input)); +} + void tools::hlsl::MetalConverter::ConstructJob( Compilation &C, const JobAction &JA, const InputInfo &Output, const InputInfoList &Inputs, const ArgList &Args, @@ -384,12 +401,20 @@ HLSLToolChain::HLSLToolChain(const Driver &D, const llvm::Triple &Triple, if (Args.hasArg(options::OPT_dxc_validator_path_EQ)) getProgramPaths().push_back( Args.getLastArgValue(options::OPT_dxc_validator_path_EQ).str()); + if (Args.hasArg(options::OPT_spirv_validator_path_EQ)) + getProgramPaths().push_back( + Args.getLastArgValue(options::OPT_spirv_validator_path_EQ).str()); } Tool *clang::driver::toolchains::HLSLToolChain::getTool( Action::ActionClass AC) const { switch (AC) { case Action::BinaryAnalyzeJobClass: + if (getTriple().isSPIRV()) { + if (!SPIRVValidator) + SPIRVValidator.reset(new tools::hlsl::SPIRV_Validator(*this)); + return SPIRVValidator.get(); + } if (!Validator) Validator.reset(new tools::hlsl::Validator(*this)); return Validator.get(); @@ -574,18 +599,31 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch, return DAL; } -bool HLSLToolChain::requiresValidation(DerivedArgList &Args) const { - if (!Args.hasArg(options::OPT_dxc_Fo)) +bool HLSLToolChain::requiresValidation(DerivedArgList &Args, + bool Diagnose) const { + bool HasFo = Args.hasArg(options::OPT_dxc_Fo); + bool DisableValidation = + Args.getLastArg(options::OPT_dxc_disable_validation) != nullptr; + + if (DisableValidation || !HasFo) return false; - if (Args.getLastArg(options::OPT_dxc_disable_validation)) + if (getArch() != llvm::Triple::spirv) { + std::string DxvPath = GetProgramPath("dxv"); + if (DxvPath != "dxv") + return true; + + if (Diagnose) + getDriver().Diag(diag::warn_drv_dxc_missing_dxv); return false; + } - std::string DxvPath = GetProgramPath("dxv"); - if (DxvPath != "dxv") + std::string SpirvValPath = GetProgramPath("spirv-val"); + if (SpirvValPath != "spirv-val") return true; - getDriver().Diag(diag::warn_drv_dxc_missing_dxv); + if (Diagnose) + getDriver().Diag(diag::warn_drv_dxc_missing_spirv_val); return false; } @@ -604,8 +642,14 @@ bool HLSLToolChain::isLastJob(DerivedArgList &Args, // Note: we check in the reverse order of execution if (requiresBinaryTranslation(Args)) return AC == Action::Action::BinaryTranslatorJobClass; - if (requiresValidation(Args)) + // For SPIR-V, spirv-val is a pure validator that doesn't produce output + // files, so the compile step is the output-producing step. For DXIL, dxv + // validates and signs, producing the final output. + if (requiresValidation(Args, /*Diagnose=*/false)) { + if (getTriple().isSPIRV()) + return AC != Action::Action::BinaryAnalyzeJobClass; return AC == Action::Action::BinaryAnalyzeJobClass; + } if (requiresObjcopy(Args)) return AC == Action::Action::ObjcopyJobClass; diff --git a/clang/lib/Driver/ToolChains/HLSL.h b/clang/lib/Driver/ToolChains/HLSL.h index 5bf385e13e962..2bc592e44f84d 100644 --- a/clang/lib/Driver/ToolChains/HLSL.h +++ b/clang/lib/Driver/ToolChains/HLSL.h @@ -30,6 +30,19 @@ class LLVM_LIBRARY_VISIBILITY Validator : public Tool { const char *LinkingOutput) const override; }; +class LLVM_LIBRARY_VISIBILITY SPIRV_Validator : public Tool { +public: + SPIRV_Validator(const ToolChain &TC) + : Tool("hlsl::SPIRV_Validator", "spirv-val", 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; +}; + class LLVM_LIBRARY_VISIBILITY MetalConverter : public Tool { public: MetalConverter(const ToolChain &TC) @@ -77,7 +90,8 @@ 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; + bool requiresValidation(llvm::opt::DerivedArgList &Args, + bool Diagnose = true) const; bool requiresBinaryTranslation(llvm::opt::DerivedArgList &Args) const; bool requiresObjcopy(llvm::opt::DerivedArgList &Args) const; @@ -95,6 +109,7 @@ class LLVM_LIBRARY_VISIBILITY HLSLToolChain : public ToolChain { private: mutable std::unique_ptr<tools::hlsl::Validator> Validator; + mutable std::unique_ptr<tools::hlsl::SPIRV_Validator> SPIRVValidator; mutable std::unique_ptr<tools::hlsl::MetalConverter> MetalConverter; mutable std::unique_ptr<tools::hlsl::LLVMObjcopy> LLVMObjcopy; }; diff --git a/clang/test/Driver/dxc_spirv-val_path.hlsl b/clang/test/Driver/dxc_spirv-val_path.hlsl new file mode 100644 index 0000000000000..1ae21ca356510 --- /dev/null +++ b/clang/test/Driver/dxc_spirv-val_path.hlsl @@ -0,0 +1,26 @@ +// RUN: mkdir -p %t.dir +// RUN: env PATH="" %clang_dxc -spirv -I test -Tlib_6_3 -Fo %t.dir/a.spv -### %s 2>&1 | FileCheck %s + +// Make sure report warning, and only once. +// CHECK:spirv-val not found +// CHECK-NOT:spirv-val not found + +// RUN: echo "spirv-val" > %t.dir/spirv-val && chmod 754 %t.dir/spirv-val && %clang_dxc -spirv --spirv-val-path=%t.dir %s -Tlib_6_3 -Fo %t.dir/a.spv -### 2>&1 | FileCheck %s --check-prefix=SPIRV_VAL_PATH +// SPIRV_VAL_PATH:spirv-val{{(.exe)?}}" "{{.*}}.spv" + +// RUN: %clang_dxc -spirv -I test -Vd -Tlib_6_3 -### %s 2>&1 | FileCheck %s --check-prefix=VD +// VD:"-cc1"{{.*}}"-triple" "spirv1.6-unknown-vulkan1.3-library" +// VD-NOT:spirv-val not found + +// RUN: %clang_dxc -spirv -Tlib_6_3 -ccc-print-bindings --spirv-val-path=%t.dir -Fo %t.spv %s 2>&1 | FileCheck %s --check-prefix=BINDINGS +// BINDINGS: "spirv1.6-unknown-vulkan1.3-library" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[spv:.+]].spv" +// BINDINGS-NEXT: "spirv1.6-unknown-vulkan1.3-library" - "hlsl::SPIRV_Validator", inputs: ["[[spv]].spv"], output: "{{.+}}.obj" + +// RUN: %clang_dxc -spirv -Tlib_6_3 -ccc-print-phases --spirv-val-path=%t.dir -Fo %t.spv %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: assembler, {3}, object +// PHASES-NEXT: 5: binary-analyzer, {4}, object \ No newline at end of file >From b084fa7a5172e2502324f9df2723e22dad071e3a Mon Sep 17 00:00:00 2001 From: Joshua Batista <[email protected]> Date: Fri, 27 Mar 2026 10:39:06 -0700 Subject: [PATCH 2/5] address Finn and Steven --- clang/lib/Driver/ToolChains/HLSL.cpp | 9 ++++++++- clang/test/Driver/dxc_spirv-val_path.hlsl | 13 ++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp index be0e2ccdc911b..634e22bcf3a22 100644 --- a/clang/lib/Driver/ToolChains/HLSL.cpp +++ b/clang/lib/Driver/ToolChains/HLSL.cpp @@ -325,6 +325,13 @@ void tools::hlsl::SPIRV_Validator::ConstructJob( ArgStringList CmdArgs; assert(Inputs.size() == 1 && "Unable to handle multiple inputs."); const InputInfo &Input = Inputs[0]; + + const llvm::Triple &T = getToolChain().getTriple(); + CmdArgs.push_back("--target-env"); + CmdArgs.push_back(Args.MakeArgString(T.getOSName())); + + CmdArgs.push_back("--scalar-block-layout"); + CmdArgs.push_back(Input.getFilename()); const char *Exec = Args.MakeArgString(SPIRVValPath); @@ -608,7 +615,7 @@ bool HLSLToolChain::requiresValidation(DerivedArgList &Args, if (DisableValidation || !HasFo) return false; - if (getArch() != llvm::Triple::spirv) { + if (getArch() == llvm::Triple::dxil) { std::string DxvPath = GetProgramPath("dxv"); if (DxvPath != "dxv") return true; diff --git a/clang/test/Driver/dxc_spirv-val_path.hlsl b/clang/test/Driver/dxc_spirv-val_path.hlsl index 1ae21ca356510..4f39fa37c69e5 100644 --- a/clang/test/Driver/dxc_spirv-val_path.hlsl +++ b/clang/test/Driver/dxc_spirv-val_path.hlsl @@ -2,15 +2,18 @@ // RUN: env PATH="" %clang_dxc -spirv -I test -Tlib_6_3 -Fo %t.dir/a.spv -### %s 2>&1 | FileCheck %s // Make sure report warning, and only once. -// CHECK:spirv-val not found -// CHECK-NOT:spirv-val not found +// CHECK:spirv-val not found; resulting SPIR-V will not be validated +// CHECK-NOT:spirv-val not found; resulting SPIR-V will not be validated // RUN: echo "spirv-val" > %t.dir/spirv-val && chmod 754 %t.dir/spirv-val && %clang_dxc -spirv --spirv-val-path=%t.dir %s -Tlib_6_3 -Fo %t.dir/a.spv -### 2>&1 | FileCheck %s --check-prefix=SPIRV_VAL_PATH -// SPIRV_VAL_PATH:spirv-val{{(.exe)?}}" "{{.*}}.spv" +// SPIRV_VAL_PATH:spirv-val{{(.exe)?}}" "--target-env" "vulkan1.3" "--scalar-block-layout" "{{.*}}.spv" + +// RUN: echo "spirv-val" > %t.dir/spirv-val && chmod 754 %t.dir/spirv-val && %clang_dxc -spirv --spirv-val-path=%t.dir %s -Tlib_6_3 -fspv-target-env=vulkan1.2 -Fo %t.dir/a.spv -### 2>&1 | FileCheck %s --check-prefix=SPIRV_VAL_VK12 +// SPIRV_VAL_VK12:spirv-val{{(.exe)?}}" "--target-env" "vulkan1.2" "--scalar-block-layout" "{{.*}}.spv" // RUN: %clang_dxc -spirv -I test -Vd -Tlib_6_3 -### %s 2>&1 | FileCheck %s --check-prefix=VD // VD:"-cc1"{{.*}}"-triple" "spirv1.6-unknown-vulkan1.3-library" -// VD-NOT:spirv-val not found +// VD-NOT:spirv-val not found; resulting SPIR-V will not be validated // RUN: %clang_dxc -spirv -Tlib_6_3 -ccc-print-bindings --spirv-val-path=%t.dir -Fo %t.spv %s 2>&1 | FileCheck %s --check-prefix=BINDINGS // BINDINGS: "spirv1.6-unknown-vulkan1.3-library" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[spv:.+]].spv" @@ -23,4 +26,4 @@ // PHASES-NEXT: 2: compiler, {1}, ir // PHASES-NEXT: 3: backend, {2}, assembler // PHASES-NEXT: 4: assembler, {3}, object -// PHASES-NEXT: 5: binary-analyzer, {4}, object \ No newline at end of file +// PHASES-NEXT: 5: binary-analyzer, {4}, object >From 2dcfc33aed1d4fbc5a542fa952bbb28b2dca0c65 Mon Sep 17 00:00:00 2001 From: Joshua Batista <[email protected]> Date: Fri, 3 Apr 2026 16:11:33 -0700 Subject: [PATCH 3/5] address Justin --- clang/lib/Driver/Driver.cpp | 5 +- clang/lib/Driver/ToolChains/HLSL.cpp | 80 ++++++++++------------- clang/lib/Driver/ToolChains/HLSL.h | 29 +++----- clang/test/Driver/dxc_spirv-val_path.hlsl | 2 +- 4 files changed, 49 insertions(+), 67 deletions(-) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 70dd8285f346a..225af09c60552 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4746,8 +4746,9 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, // Call spirv-val for SPIR-V when -Vd not in Args. if (TC.requiresValidation(Args)) { Action *LastAction = Actions.back(); - Actions.push_back( - C.MakeAction<BinaryAnalyzeJobAction>(LastAction, types::TY_Object)); + if (LastAction->getType() == types::TY_Object) + Actions.push_back( + C.MakeAction<BinaryAnalyzeJobAction>(LastAction, types::TY_Object)); } } diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp index 634e22bcf3a22..d1f41c8f127cb 100644 --- a/clang/lib/Driver/ToolChains/HLSL.cpp +++ b/clang/lib/Driver/ToolChains/HLSL.cpp @@ -300,43 +300,35 @@ void tools::hlsl::Validator::ConstructJob(Compilation &C, const JobAction &JA, 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]; - CmdArgs.push_back(Input.getFilename()); - CmdArgs.push_back("-o"); - CmdArgs.push_back(Output.getFilename()); - - const char *Exec = Args.MakeArgString(DxvPath); - C.addCommand(std::make_unique<Command>(JA, *this, ResponseFileSupport::None(), - Exec, CmdArgs, Inputs, Input)); -} - -void tools::hlsl::SPIRV_Validator::ConstructJob( - Compilation &C, const JobAction &JA, const InputInfo &Output, - const InputInfoList &Inputs, const ArgList &Args, - const char *LinkingOutput) const { - std::string SPIRVValPath = getToolChain().GetProgramPath("spirv-val"); - assert(SPIRVValPath != "spirv-val" && "cannot find spirv-val"); - ArgStringList CmdArgs; assert(Inputs.size() == 1 && "Unable to handle multiple inputs."); const InputInfo &Input = Inputs[0]; const llvm::Triple &T = getToolChain().getTriple(); - CmdArgs.push_back("--target-env"); - CmdArgs.push_back(Args.MakeArgString(T.getOSName())); - - CmdArgs.push_back("--scalar-block-layout"); - - CmdArgs.push_back(Input.getFilename()); - - const char *Exec = Args.MakeArgString(SPIRVValPath); - C.addCommand(std::make_unique<Command>(JA, *this, ResponseFileSupport::None(), - Exec, CmdArgs, Inputs, Input)); + if (T.isSPIRV()) { + std::string SPIRVValPath = getToolChain().GetProgramPath("spirv-val"); + assert(SPIRVValPath != "spirv-val" && "cannot find spirv-val"); + + CmdArgs.push_back("--target-env"); + CmdArgs.push_back(Args.MakeArgString(T.getOSName())); + CmdArgs.push_back("--scalar-block-layout"); + CmdArgs.push_back(Input.getFilename()); + + const char *Exec = Args.MakeArgString(SPIRVValPath); + C.addCommand(std::make_unique<Command>( + JA, *this, ResponseFileSupport::None(), Exec, CmdArgs, Inputs, Input)); + } else { + std::string DxvPath = getToolChain().GetProgramPath("dxv"); + assert(DxvPath != "dxv" && "cannot find dxv"); + + CmdArgs.push_back(Input.getFilename()); + CmdArgs.push_back("-o"); + CmdArgs.push_back(Output.getFilename()); + + const char *Exec = Args.MakeArgString(DxvPath); + C.addCommand(std::make_unique<Command>( + JA, *this, ResponseFileSupport::None(), Exec, CmdArgs, Inputs, Input)); + } } void tools::hlsl::MetalConverter::ConstructJob( @@ -417,11 +409,6 @@ Tool *clang::driver::toolchains::HLSLToolChain::getTool( Action::ActionClass AC) const { switch (AC) { case Action::BinaryAnalyzeJobClass: - if (getTriple().isSPIRV()) { - if (!SPIRVValidator) - SPIRVValidator.reset(new tools::hlsl::SPIRV_Validator(*this)); - return SPIRVValidator.get(); - } if (!Validator) Validator.reset(new tools::hlsl::Validator(*this)); return Validator.get(); @@ -625,12 +612,15 @@ bool HLSLToolChain::requiresValidation(DerivedArgList &Args, return false; } - std::string SpirvValPath = GetProgramPath("spirv-val"); - if (SpirvValPath != "spirv-val") - return true; + if (getTriple().isSPIRV()) { + std::string SpirvValPath = GetProgramPath("spirv-val"); + if (SpirvValPath != "spirv-val") + return true; + + if (Diagnose) + getDriver().Diag(diag::warn_drv_dxc_missing_spirv_val); + } - if (Diagnose) - getDriver().Diag(diag::warn_drv_dxc_missing_spirv_val); return false; } @@ -650,11 +640,11 @@ bool HLSLToolChain::isLastJob(DerivedArgList &Args, if (requiresBinaryTranslation(Args)) return AC == Action::Action::BinaryTranslatorJobClass; // For SPIR-V, spirv-val is a pure validator that doesn't produce output - // files, so the compile step is the output-producing step. For DXIL, dxv - // validates and signs, producing the final output. + // files, so the compile step is the last output-producing job. For DXIL, + // dxv validates and signs, producing the final output. if (requiresValidation(Args, /*Diagnose=*/false)) { if (getTriple().isSPIRV()) - return AC != Action::Action::BinaryAnalyzeJobClass; + return AC == Action::Action::AssembleJobClass; return AC == Action::Action::BinaryAnalyzeJobClass; } if (requiresObjcopy(Args)) diff --git a/clang/lib/Driver/ToolChains/HLSL.h b/clang/lib/Driver/ToolChains/HLSL.h index 2bc592e44f84d..ced551a4aa24c 100644 --- a/clang/lib/Driver/ToolChains/HLSL.h +++ b/clang/lib/Driver/ToolChains/HLSL.h @@ -20,20 +20,9 @@ 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; -}; - -class LLVM_LIBRARY_VISIBILITY SPIRV_Validator : public Tool { -public: - SPIRV_Validator(const ToolChain &TC) - : Tool("hlsl::SPIRV_Validator", "spirv-val", TC) {} + Validator(const ToolChain &TC) + : Tool("hlsl::Validator", TC.getTriple().isSPIRV() ? "spirv-val" : "dxv", + TC) {} bool hasIntegratedCPP() const override { return false; } @@ -95,11 +84,14 @@ class LLVM_LIBRARY_VISIBILITY HLSLToolChain : public ToolChain { bool requiresBinaryTranslation(llvm::opt::DerivedArgList &Args) const; bool requiresObjcopy(llvm::opt::DerivedArgList &Args) const; - /// If we are targeting DXIL then the last job should output the DXContainer - /// to the specified output file with /Fo. Otherwise, we will emit to a - /// temporary file for the next job to use. + /// Determines whether the given action class is the last job that produces + /// an output file. This is used to decide whether to write to the -Fo + /// output path or to a temporary file. /// - /// Returns true if we should output to the final result file. + /// For example, spirv-val is a pure validator that runs after the compile + /// step but doesn't produce output, so the compile step is the last + /// output-producing job. For DXIL, dxv validates and signs, producing the + /// final output. bool isLastJob(llvm::opt::DerivedArgList &Args, Action::ActionClass AC) const; // Set default DWARF version to 4 for DXIL uses version 4. @@ -109,7 +101,6 @@ class LLVM_LIBRARY_VISIBILITY HLSLToolChain : public ToolChain { private: mutable std::unique_ptr<tools::hlsl::Validator> Validator; - mutable std::unique_ptr<tools::hlsl::SPIRV_Validator> SPIRVValidator; mutable std::unique_ptr<tools::hlsl::MetalConverter> MetalConverter; mutable std::unique_ptr<tools::hlsl::LLVMObjcopy> LLVMObjcopy; }; diff --git a/clang/test/Driver/dxc_spirv-val_path.hlsl b/clang/test/Driver/dxc_spirv-val_path.hlsl index 4f39fa37c69e5..4a5090fc56863 100644 --- a/clang/test/Driver/dxc_spirv-val_path.hlsl +++ b/clang/test/Driver/dxc_spirv-val_path.hlsl @@ -17,7 +17,7 @@ // RUN: %clang_dxc -spirv -Tlib_6_3 -ccc-print-bindings --spirv-val-path=%t.dir -Fo %t.spv %s 2>&1 | FileCheck %s --check-prefix=BINDINGS // BINDINGS: "spirv1.6-unknown-vulkan1.3-library" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[spv:.+]].spv" -// BINDINGS-NEXT: "spirv1.6-unknown-vulkan1.3-library" - "hlsl::SPIRV_Validator", inputs: ["[[spv]].spv"], output: "{{.+}}.obj" +// BINDINGS-NEXT: "spirv1.6-unknown-vulkan1.3-library" - "hlsl::Validator", inputs: ["[[spv]].spv"], output: "{{.+}}.obj" // RUN: %clang_dxc -spirv -Tlib_6_3 -ccc-print-phases --spirv-val-path=%t.dir -Fo %t.spv %s 2>&1 | FileCheck %s --check-prefix=PHASES >From 940d8539849fffcf51539dad31a87026cc010f76 Mon Sep 17 00:00:00 2001 From: Joshua Batista <[email protected]> Date: Fri, 3 Apr 2026 16:53:28 -0700 Subject: [PATCH 4/5] account for spirv include tests edge case not found failure --- clang/test/Driver/dxc_spirv-val_missing.hlsl | 15 +++++++++++++++ clang/test/Driver/dxc_spirv-val_path.hlsl | 9 --------- clang/test/lit.cfg.py | 4 ++++ 3 files changed, 19 insertions(+), 9 deletions(-) create mode 100644 clang/test/Driver/dxc_spirv-val_missing.hlsl diff --git a/clang/test/Driver/dxc_spirv-val_missing.hlsl b/clang/test/Driver/dxc_spirv-val_missing.hlsl new file mode 100644 index 0000000000000..c284d71203d80 --- /dev/null +++ b/clang/test/Driver/dxc_spirv-val_missing.hlsl @@ -0,0 +1,15 @@ +// UNSUPPORTED: spirv-val +// +// Verify that a warning is emitted exactly once when spirv-val is not found. +// This test is unsupported when spirv-val is in the build directory because +// GetProgramPath finds it via the application directory search, bypassing PATH. + +// RUN: env PATH="" %clang_dxc -spirv -I test -Tlib_6_3 -Fo %t.spv -### %s 2>&1 | FileCheck %s + +// CHECK: spirv-val not found; resulting SPIR-V will not be validated +// CHECK-NOT: spirv-val not found; resulting SPIR-V will not be validated + +// Also verify -Vd suppresses the warning. +// RUN: %clang_dxc -spirv -I test -Vd -Tlib_6_3 -### %s 2>&1 | FileCheck %s --check-prefix=VD +// VD: "-cc1"{{.*}}"-triple" "spirv1.6-unknown-vulkan1.3-library" +// VD-NOT: spirv-val not found; resulting SPIR-V will not be validated diff --git a/clang/test/Driver/dxc_spirv-val_path.hlsl b/clang/test/Driver/dxc_spirv-val_path.hlsl index 4a5090fc56863..1fed1d01d4b18 100644 --- a/clang/test/Driver/dxc_spirv-val_path.hlsl +++ b/clang/test/Driver/dxc_spirv-val_path.hlsl @@ -1,9 +1,4 @@ // RUN: mkdir -p %t.dir -// RUN: env PATH="" %clang_dxc -spirv -I test -Tlib_6_3 -Fo %t.dir/a.spv -### %s 2>&1 | FileCheck %s - -// Make sure report warning, and only once. -// CHECK:spirv-val not found; resulting SPIR-V will not be validated -// CHECK-NOT:spirv-val not found; resulting SPIR-V will not be validated // RUN: echo "spirv-val" > %t.dir/spirv-val && chmod 754 %t.dir/spirv-val && %clang_dxc -spirv --spirv-val-path=%t.dir %s -Tlib_6_3 -Fo %t.dir/a.spv -### 2>&1 | FileCheck %s --check-prefix=SPIRV_VAL_PATH // SPIRV_VAL_PATH:spirv-val{{(.exe)?}}" "--target-env" "vulkan1.3" "--scalar-block-layout" "{{.*}}.spv" @@ -11,10 +6,6 @@ // RUN: echo "spirv-val" > %t.dir/spirv-val && chmod 754 %t.dir/spirv-val && %clang_dxc -spirv --spirv-val-path=%t.dir %s -Tlib_6_3 -fspv-target-env=vulkan1.2 -Fo %t.dir/a.spv -### 2>&1 | FileCheck %s --check-prefix=SPIRV_VAL_VK12 // SPIRV_VAL_VK12:spirv-val{{(.exe)?}}" "--target-env" "vulkan1.2" "--scalar-block-layout" "{{.*}}.spv" -// RUN: %clang_dxc -spirv -I test -Vd -Tlib_6_3 -### %s 2>&1 | FileCheck %s --check-prefix=VD -// VD:"-cc1"{{.*}}"-triple" "spirv1.6-unknown-vulkan1.3-library" -// VD-NOT:spirv-val not found; resulting SPIR-V will not be validated - // RUN: %clang_dxc -spirv -Tlib_6_3 -ccc-print-bindings --spirv-val-path=%t.dir -Fo %t.spv %s 2>&1 | FileCheck %s --check-prefix=BINDINGS // BINDINGS: "spirv1.6-unknown-vulkan1.3-library" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[spv:.+]].spv" // BINDINGS-NEXT: "spirv1.6-unknown-vulkan1.3-library" - "hlsl::Validator", inputs: ["[[spv]].spv"], output: "{{.+}}.obj" diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py index 3b0b3092d424e..4de86f555bcb8 100644 --- a/clang/test/lit.cfg.py +++ b/clang/test/lit.cfg.py @@ -268,6 +268,10 @@ def have_host_clang_repl_cuda(): if config.clang_enable_cir: config.available_features.add("cir-support") +# SPIRV-Tools validator availability (e.g. built with -DLLVM_INCLUDE_SPIRV_TOOLS_TESTS) +if lit.util.which("spirv-val", config.llvm_tools_dir): + config.available_features.add("spirv-val") + llvm_config.add_tool_substitutions(tools, tool_dirs) config.substitutions.append( >From c1d5c32adf62057e0723763e669bb3cd2d7a8eb4 Mon Sep 17 00:00:00 2001 From: Joshua Batista <[email protected]> Date: Tue, 7 Apr 2026 18:29:35 -0700 Subject: [PATCH 5/5] address Justin --- clang/lib/Driver/Driver.cpp | 60 ++++++++---------------- clang/lib/Driver/ToolChains/HLSL.cpp | 69 +++++++++++++++------------- clang/lib/Driver/ToolChains/HLSL.h | 16 +++++-- 3 files changed, 68 insertions(+), 77 deletions(-) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 225af09c60552..577b06a31a04d 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4705,7 +4705,8 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, } } - if (C.getDefaultToolChain().getTriple().isDXIL()) { + if (C.getDefaultToolChain().getTriple().isDXIL() || + C.getDefaultToolChain().getTriple().isSPIRV()) { const auto &TC = static_cast<const toolchains::HLSLToolChain &>(C.getDefaultToolChain()); @@ -4719,11 +4720,16 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, C.MakeAction<ObjcopyJobAction>(LastAction, types::TY_Object)); } - // Call validator for dxil when -Vd not in Args. - if (TC.requiresValidation(Args)) { + // Call validator when -Vd not in Args. + auto ValInfo = TC.getValidationInfo(Args); + if (ValInfo.NeedsValidation) { Action *LastAction = Actions.back(); - Actions.push_back(C.MakeAction<BinaryAnalyzeJobAction>( - LastAction, types::TY_DX_CONTAINER)); + if (LastAction->getType() == types::TY_Object) { + types::ID OutType = + ValInfo.ProducesOutput ? types::TY_DX_CONTAINER : types::TY_Object; + Actions.push_back( + C.MakeAction<BinaryAnalyzeJobAction>(LastAction, OutType)); + } } // Call metal-shaderconverter when targeting metal. @@ -4739,19 +4745,6 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, } } - if (C.getDefaultToolChain().getTriple().isSPIRV()) { - const auto &TC = - static_cast<const toolchains::HLSLToolChain &>(C.getDefaultToolChain()); - - // Call spirv-val for SPIR-V when -Vd not in Args. - if (TC.requiresValidation(Args)) { - Action *LastAction = Actions.back(); - if (LastAction->getType() == types::TY_Object) - Actions.push_back( - C.MakeAction<BinaryAnalyzeJobAction>(LastAction, types::TY_Object)); - } - } - // Claim ignored clang-cl options. Args.ClaimAllArgs(options::OPT_cl_ignored_Group); } @@ -6430,33 +6423,16 @@ const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA, C.getArgs().hasArg(options::OPT_dxc_Fo)) || JA.getType() == types::TY_DX_CONTAINER) { StringRef FoValue = C.getArgs().getLastArgValue(options::OPT_dxc_Fo); - // If we are targeting DXIL and not validating/translating/objcopying, we - // should set the final result file. Otherwise we should emit to a - // temporary. - if (C.getDefaultToolChain().getTriple().isDXIL()) { - const auto &TC = static_cast<const toolchains::HLSLToolChain &>( - C.getDefaultToolChain()); - // Fo can be empty here if the validator is running for a compiler flow - // that is using Fc or just printing disassembly. - if (TC.isLastJob(C.getArgs(), JA.getKind()) && !FoValue.empty()) - return C.addResultFile(C.getArgs().MakeArgString(FoValue.str()), &JA); - StringRef Name = llvm::sys::path::filename(BaseInput); - std::pair<StringRef, StringRef> Split = Name.split('.'); - const char *Suffix = types::getTypeTempSuffix(JA.getType(), true); - return CreateTempFile(C, Split.first, Suffix, false); - } - - assert(C.getDefaultToolChain().getTriple().isSPIRV()); + assert((C.getDefaultToolChain().getTriple().isDXIL() || + C.getDefaultToolChain().getTriple().isSPIRV()) && + "expected DXIL or SPIR-V triple for HLSL output path"); const auto &TC = static_cast<const toolchains::HLSLToolChain &>(C.getDefaultToolChain()); - - // If this is the last job in the compilation for this input, and Fo is - // non-empty, we can return the Fo file (the final output) - if (TC.isLastJob(C.getArgs(), JA.getKind()) && !FoValue.empty()) + // Fo can be empty here if the validator is running for a compiler flow + // that is using Fc or just printing disassembly. + if (TC.isLastOutputProducingJob(C.getArgs(), JA.getKind()) && + !FoValue.empty()) return C.addResultFile(C.getArgs().MakeArgString(FoValue.str()), &JA); - - // Otherwise, create a temporary file for validation (like DXIL creates - // temp files). StringRef Name = llvm::sys::path::filename(BaseInput); std::pair<StringRef, StringRef> Split = Name.split('.'); const char *Suffix = types::getTypeTempSuffix(JA.getType(), true); diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp index d1f41c8f127cb..834b8acc78734 100644 --- a/clang/lib/Driver/ToolChains/HLSL.cpp +++ b/clang/lib/Driver/ToolChains/HLSL.cpp @@ -305,30 +305,29 @@ void tools::hlsl::Validator::ConstructJob(Compilation &C, const JobAction &JA, const InputInfo &Input = Inputs[0]; const llvm::Triple &T = getToolChain().getTriple(); + std::string ExecPath; if (T.isSPIRV()) { - std::string SPIRVValPath = getToolChain().GetProgramPath("spirv-val"); - assert(SPIRVValPath != "spirv-val" && "cannot find spirv-val"); + ExecPath = getToolChain().GetProgramPath("spirv-val"); + assert(ExecPath != "spirv-val" && "cannot find spirv-val"); CmdArgs.push_back("--target-env"); CmdArgs.push_back(Args.MakeArgString(T.getOSName())); CmdArgs.push_back("--scalar-block-layout"); CmdArgs.push_back(Input.getFilename()); - - const char *Exec = Args.MakeArgString(SPIRVValPath); - C.addCommand(std::make_unique<Command>( - JA, *this, ResponseFileSupport::None(), Exec, CmdArgs, Inputs, Input)); - } else { - std::string DxvPath = getToolChain().GetProgramPath("dxv"); - assert(DxvPath != "dxv" && "cannot find dxv"); + } else if (T.isDXIL()) { + ExecPath = getToolChain().GetProgramPath("dxv"); + assert(ExecPath != "dxv" && "cannot find dxv"); CmdArgs.push_back(Input.getFilename()); CmdArgs.push_back("-o"); CmdArgs.push_back(Output.getFilename()); - - const char *Exec = Args.MakeArgString(DxvPath); - C.addCommand(std::make_unique<Command>( - JA, *this, ResponseFileSupport::None(), Exec, CmdArgs, Inputs, Input)); + } else { + llvm_unreachable("unexpected triple for HLSL validation"); } + + const char *Exec = Args.MakeArgString(ExecPath); + C.addCommand(std::make_unique<Command>(JA, *this, ResponseFileSupport::None(), + Exec, CmdArgs, Inputs, Input)); } void tools::hlsl::MetalConverter::ConstructJob( @@ -593,35 +592,43 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch, return DAL; } -bool HLSLToolChain::requiresValidation(DerivedArgList &Args, - bool Diagnose) const { +HLSLToolChain::ValidationInfo +HLSLToolChain::getValidationInfo(DerivedArgList &Args, bool Diagnose) const { + ValidationInfo Info; + bool HasFo = Args.hasArg(options::OPT_dxc_Fo); bool DisableValidation = Args.getLastArg(options::OPT_dxc_disable_validation) != nullptr; if (DisableValidation || !HasFo) - return false; + return Info; - if (getArch() == llvm::Triple::dxil) { + if (getTriple().isDXIL()) { std::string DxvPath = GetProgramPath("dxv"); - if (DxvPath != "dxv") - return true; + if (DxvPath != "dxv") { + Info.NeedsValidation = true; + Info.ProducesOutput = true; + return Info; + } if (Diagnose) getDriver().Diag(diag::warn_drv_dxc_missing_dxv); - return false; + return Info; } if (getTriple().isSPIRV()) { std::string SpirvValPath = GetProgramPath("spirv-val"); - if (SpirvValPath != "spirv-val") - return true; + if (SpirvValPath != "spirv-val") { + Info.NeedsValidation = true; + Info.ProducesOutput = false; + return Info; + } if (Diagnose) getDriver().Diag(diag::warn_drv_dxc_missing_spirv_val); } - return false; + return Info; } bool HLSLToolChain::requiresBinaryTranslation(DerivedArgList &Args) const { @@ -634,18 +641,16 @@ bool HLSLToolChain::requiresObjcopy(DerivedArgList &Args) const { Args.hasArg(options::OPT_dxc_Frs) || isRootSignatureTarget(Args)); } -bool HLSLToolChain::isLastJob(DerivedArgList &Args, - Action::ActionClass AC) const { +bool HLSLToolChain::isLastOutputProducingJob(DerivedArgList &Args, + Action::ActionClass AC) const { // Note: we check in the reverse order of execution if (requiresBinaryTranslation(Args)) return AC == Action::Action::BinaryTranslatorJobClass; - // For SPIR-V, spirv-val is a pure validator that doesn't produce output - // files, so the compile step is the last output-producing job. For DXIL, - // dxv validates and signs, producing the final output. - if (requiresValidation(Args, /*Diagnose=*/false)) { - if (getTriple().isSPIRV()) - return AC == Action::Action::AssembleJobClass; - return AC == Action::Action::BinaryAnalyzeJobClass; + auto ValInfo = getValidationInfo(Args, /*Diagnose=*/false); + if (ValInfo.NeedsValidation) { + if (ValInfo.ProducesOutput) + return AC == Action::Action::BinaryAnalyzeJobClass; + return AC == Action::Action::AssembleJobClass; } if (requiresObjcopy(Args)) return AC == Action::Action::ObjcopyJobClass; diff --git a/clang/lib/Driver/ToolChains/HLSL.h b/clang/lib/Driver/ToolChains/HLSL.h index ced551a4aa24c..0806c252c1073 100644 --- a/clang/lib/Driver/ToolChains/HLSL.h +++ b/clang/lib/Driver/ToolChains/HLSL.h @@ -79,8 +79,17 @@ 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, - bool Diagnose = true) const; + + struct ValidationInfo { + bool NeedsValidation = false; + bool ProducesOutput = false; + }; + + /// Returns information about whether validation is required and whether the + /// validator produces output. When Diagnose is true, emits a warning if the + /// required validator executable cannot be found. + ValidationInfo getValidationInfo(llvm::opt::DerivedArgList &Args, + bool Diagnose = true) const; bool requiresBinaryTranslation(llvm::opt::DerivedArgList &Args) const; bool requiresObjcopy(llvm::opt::DerivedArgList &Args) const; @@ -92,7 +101,8 @@ class LLVM_LIBRARY_VISIBILITY HLSLToolChain : public ToolChain { /// step but doesn't produce output, so the compile step is the last /// output-producing job. For DXIL, dxv validates and signs, producing the /// final output. - bool isLastJob(llvm::opt::DerivedArgList &Args, Action::ActionClass AC) const; + bool isLastOutputProducingJob(llvm::opt::DerivedArgList &Args, + Action::ActionClass AC) const; // Set default DWARF version to 4 for DXIL uses version 4. unsigned GetDefaultDwarfVersion() const override { return 4; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
