jhuber6 added inline comments.
================
Comment at: clang/lib/Driver/Driver.cpp:4216
+ llvm::Triple T(getTargetTriple());
+ if (T.getArch() == llvm::Triple::dxil &&
!Args.getLastArg(options::OPT_dxc_disable_validation)) {
+ // Only add action when 'dxv' exists.
----------------
Could we move this logic into the HLSL ToolChain like we do with CUDA / ROCm?
You should be able to then cast the toolchain to `HLSLToolChain` and query it
here.
================
Comment at: clang/lib/Driver/Driver.cpp:4217
+ if (T.getArch() == llvm::Triple::dxil &&
!Args.getLastArg(options::OPT_dxc_disable_validation)) {
+ // Only add action when 'dxv' exists.
+ std::string DxvPath = GetProgramPath("dxv", C.getDefaultToolChain());
----------------
================
Comment at: clang/lib/Driver/Driver.cpp:4222
+ Actions.push_back(
+ C.MakeAction<BinaryAnalyzeJobAction>(LastAction, types::TY_Nothing));
+ } else {
----------------
Why is the type `TY_Nothing`? The `ConstructJob` invocation shows it using
`-o`. So it should have output?
================
Comment at: clang/lib/Driver/Driver.cpp:4224
+ } else {
+ // Not find dxv.
+ C.getDriver().Diag(diag::warn_drv_dxc_missing_dxv);
----------------
================
Comment at: clang/lib/Driver/ToolChains/HLSL.cpp:171
+ // discover the dxv executable.
+ getProgramPaths().push_back(getDriver().Dir);
+}
----------------
Just to check since I'm not really familiar at all with this toolchain, but
does `dxv` exist as a clang tool? This path exists to search in the
`/path/to/llvm/install/bin` directory. If it's an external binary this
shouldn't be necessary.
================
Comment at: clang/test/Driver/dxc_dxv_path.hlsl:11
+// VD-NOT:dxv not found
+// VD:"-triple" "dxil-unknown-shadermodel6.3-library"
+
----------------
Lines that check clang typically start with `"-cc1"`
================
Comment at: clang/test/Driver/dxc_dxv_path.hlsl:14-15
+// RUN: %clang_dxc -Tlib_6_3 -ccc-print-bindings --dxv-path=%T -Fo %t.dxc %s
2>&1 | FileCheck %s --check-prefix=BINDINGS
+// BINDINGS: "dxil-unknown-shadermodel6.3-library" - "clang", inputs:
["{{.*}}dxc_dxv_path.hlsl"], output: "[[DXC:.+]].dxc"
+// BINDINGS: "dxil-unknown-shadermodel6.3-library" - "hlsl::Validator",
inputs: ["[[DXC]].dxc"], output: (nothing)
----------------
Can we get a test for `-ccc-print-phases` as well? Also, I wouldn't bother
checking the test file's name and it's good practice to stipulate `NEXT` on
these kinds of tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141705/new/
https://reviews.llvm.org/D141705
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
