From fde240c9c328b8a824a064c8ccd91252516e6d41 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Fri, 13 May 2022 14:36:27 -0700 Subject: [PATCH] [HLSL][clang][Driver] Parse target profile early to update Driver::TargetTriple. This is to avoid err_target_unknown_abi which is caused by use default TargetTriple instead of shader model target triple. Reviewed By: beanz Differential Revision: https://reviews.llvm.org/D125585 --- .../clang/Basic/DiagnosticDriverKinds.td | 2 + clang/lib/Driver/Driver.cpp | 18 +- clang/lib/Driver/ToolChains/HLSL.cpp | 31 ++-- clang/lib/Driver/ToolChains/HLSL.h | 4 +- clang/test/Driver/dxc_fcgl.hlsl | 2 +- clang/unittests/Driver/ToolChainTest.cpp | 157 ++++++------------ 6 files changed, 82 insertions(+), 132 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index 9f4864a33a33..618e748da444 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -666,6 +666,8 @@ def err_drv_target_variant_invalid : Error< def err_drv_invalid_directx_shader_module : Error< "invalid profile : %0">; +def err_drv_dxc_missing_target_profile : Error< + "target profile option (-T) is missing">; def err_drv_invalid_range_dxil_validator_version : Error< "invalid validator version : %0\n" diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index aaef2e1ded32..3185ebcd46a4 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1236,11 +1236,19 @@ Compilation *Driver::BuildCompilation(ArrayRef ArgList) { T.setObjectFormat(llvm::Triple::COFF); TargetTriple = T.str(); } else if (IsDXCMode()) { - // clang-dxc target is build from target_profile option. - // Just set OS to shader model to select HLSLToolChain. - llvm::Triple T(TargetTriple); - T.setOS(llvm::Triple::ShaderModel); - TargetTriple = T.str(); + // Build TargetTriple from target_profile option for clang-dxc. + if (const Arg *A = Args.getLastArg(options::OPT_target_profile)) { + StringRef TargetProfile = A->getValue(); + if (auto Triple = + toolchains::HLSLToolChain::parseTargetProfile(TargetProfile)) + TargetTriple = Triple.getValue(); + else + Diag(diag::err_drv_invalid_directx_shader_module) << TargetProfile; + + A->claim(); + } else { + Diag(diag::err_drv_dxc_missing_target_profile); + } } if (const Arg *A = Args.getLastArg(options::OPT_target)) diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp index b62395b168ea..c2b55fdde4b4 100644 --- a/clang/lib/Driver/ToolChains/HLSL.cpp +++ b/clang/lib/Driver/ToolChains/HLSL.cpp @@ -64,12 +64,12 @@ bool isLegalShaderModel(Triple &T) { return false; } -std::string tryParseProfile(StringRef Profile) { +llvm::Optional tryParseProfile(StringRef Profile) { // [ps|vs|gs|hs|ds|cs|ms|as]_[major]_[minor] SmallVector Parts; Profile.split(Parts, "_"); if (Parts.size() != 3) - return ""; + return NoneType(); Triple::EnvironmentType Kind = StringSwitch(Parts[0]) @@ -84,17 +84,17 @@ std::string tryParseProfile(StringRef Profile) { .Case("as", Triple::EnvironmentType::Amplification) .Default(Triple::EnvironmentType::UnknownEnvironment); if (Kind == Triple::EnvironmentType::UnknownEnvironment) - return ""; + return NoneType(); unsigned long long Major = 0; if (llvm::getAsUnsignedInteger(Parts[1], 0, Major)) - return ""; + return NoneType(); unsigned long long Minor = 0; if (Parts[2] == "x" && Kind == Triple::EnvironmentType::Library) Minor = OfflineLibMinor; else if (llvm::getAsUnsignedInteger(Parts[2], 0, Minor)) - return ""; + return NoneType(); // dxil-unknown-shadermodel-hull llvm::Triple T; @@ -105,7 +105,7 @@ std::string tryParseProfile(StringRef Profile) { if (isLegalShaderModel(T)) return T.getTriple(); else - return ""; + return NoneType(); } bool isLegalValidatorVersion(StringRef ValVersionStr, const Driver &D) { @@ -138,21 +138,10 @@ HLSLToolChain::HLSLToolChain(const Driver &D, const llvm::Triple &Triple, const ArgList &Args) : ToolChain(D, Triple, Args) {} -std::string -HLSLToolChain::ComputeEffectiveClangTriple(const ArgList &Args, - types::ID InputType) const { - if (Arg *A = Args.getLastArg(options::OPT_target_profile)) { - StringRef Profile = A->getValue(); - std::string Triple = tryParseProfile(Profile); - if (Triple == "") { - getDriver().Diag(diag::err_drv_invalid_directx_shader_module) << Profile; - Triple = ToolChain::ComputeEffectiveClangTriple(Args, InputType); - } - A->claim(); - return Triple; - } else { - return ToolChain::ComputeEffectiveClangTriple(Args, InputType); - } +llvm::Optional +clang::driver::toolchains::HLSLToolChain::parseTargetProfile( + StringRef TargetProfile) { + return tryParseProfile(TargetProfile); } DerivedArgList * diff --git a/clang/lib/Driver/ToolChains/HLSL.h b/clang/lib/Driver/ToolChains/HLSL.h index 7774db3762dd..5573b0cc69e2 100644 --- a/clang/lib/Driver/ToolChains/HLSL.h +++ b/clang/lib/Driver/ToolChains/HLSL.h @@ -29,8 +29,8 @@ public: llvm::opt::DerivedArgList * TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) const override; - std::string ComputeEffectiveClangTriple(const llvm::opt::ArgList &Args, - types::ID InputType) const override; + static llvm::Optional + parseTargetProfile(StringRef TargetProfile); }; } // end namespace toolchains diff --git a/clang/test/Driver/dxc_fcgl.hlsl b/clang/test/Driver/dxc_fcgl.hlsl index d3eb2523199c..782dfaa1e0e5 100644 --- a/clang/test/Driver/dxc_fcgl.hlsl +++ b/clang/test/Driver/dxc_fcgl.hlsl @@ -1,4 +1,4 @@ -// RUN: %clang_dxc -fcgl foo.hlsl -### %s 2>&1 | FileCheck %s +// RUN: %clang_dxc -fcgl -T lib_6_7 foo.hlsl -### %s 2>&1 | FileCheck %s // Make sure fcgl option flag which translated into "-S" "-emit-llvm" "-disable-llvm-passes". // CHECK:"-S" "-emit-llvm" "-disable-llvm-passes" diff --git a/clang/unittests/Driver/ToolChainTest.cpp b/clang/unittests/Driver/ToolChainTest.cpp index c652b093dc99..f412c3569554 100644 --- a/clang/unittests/Driver/ToolChainTest.cpp +++ b/clang/unittests/Driver/ToolChainTest.cpp @@ -387,6 +387,28 @@ struct SimpleDiagnosticConsumer : public DiagnosticConsumer { std::vector> Errors; }; +static void validateTargetProfile(StringRef TargetProfile, + StringRef ExpectTriple, Driver &TheDriver, + DiagnosticsEngine &Diags) { + EXPECT_TRUE(TheDriver.BuildCompilation( + {"clang", "--driver-mode=dxc", TargetProfile.data(), "foo.hlsl"})); + EXPECT_STREQ(TheDriver.getTargetTriple().c_str(), ExpectTriple.data()); + EXPECT_EQ(Diags.getNumErrors(), 0u); +} + +static void validateTargetProfile(StringRef TargetProfile, + StringRef ExpectError, Driver &TheDriver, + DiagnosticsEngine &Diags, + SimpleDiagnosticConsumer *DiagConsumer, + unsigned NumOfErrors) { + EXPECT_TRUE(TheDriver.BuildCompilation( + {"clang", "--driver-mode=dxc", TargetProfile.data(), "foo.hlsl"})); + EXPECT_EQ(Diags.getNumErrors(), NumOfErrors); + EXPECT_STREQ(DiagConsumer->Errors.back().c_str(), ExpectError.data()); + Diags.Clear(); + DiagConsumer->clear(); +} + TEST(DxcModeTest, TargetProfileValidation) { IntrusiveRefCntPtr DiagID(new DiagnosticIDs()); @@ -400,111 +422,40 @@ TEST(DxcModeTest, TargetProfileValidation) { IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions(); DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer); Driver TheDriver("/bin/clang", "", Diags, "", InMemoryFileSystem); - std::unique_ptr C( - TheDriver.BuildCompilation({"clang", "--driver-mode=dxc", "foo.hlsl"})); - EXPECT_TRUE(C); - EXPECT_TRUE(!C->containsError()); - auto &TC = C->getDefaultToolChain(); - bool ContainsError = false; - auto Args = TheDriver.ParseArgStrings({"-Tvs_6_0"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - auto Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.0-vertex"); - EXPECT_EQ(Diags.getNumErrors(), 0u); - - Args = TheDriver.ParseArgStrings({"-Ths_6_1"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.1-hull"); - EXPECT_EQ(Diags.getNumErrors(), 0u); - - Args = TheDriver.ParseArgStrings({"-Tds_6_2"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.2-domain"); - EXPECT_EQ(Diags.getNumErrors(), 0u); - - Args = TheDriver.ParseArgStrings({"-Tds_6_2"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.2-domain"); - EXPECT_EQ(Diags.getNumErrors(), 0u); - - Args = TheDriver.ParseArgStrings({"-Tgs_6_3"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.3-geometry"); - EXPECT_EQ(Diags.getNumErrors(), 0u); - - Args = TheDriver.ParseArgStrings({"-Tps_6_4"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.4-pixel"); - EXPECT_EQ(Diags.getNumErrors(), 0u); - - Args = TheDriver.ParseArgStrings({"-Tcs_6_5"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.5-compute"); - EXPECT_EQ(Diags.getNumErrors(), 0u); - - Args = TheDriver.ParseArgStrings({"-Tms_6_6"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.6-mesh"); - EXPECT_EQ(Diags.getNumErrors(), 0u); - - Args = TheDriver.ParseArgStrings({"-Tas_6_7"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.7-amplification"); - EXPECT_EQ(Diags.getNumErrors(), 0u); - - Args = TheDriver.ParseArgStrings({"-Tlib_6_x"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.15-library"); - EXPECT_EQ(Diags.getNumErrors(), 0u); + validateTargetProfile("-Tvs_6_0", "dxil--shadermodel6.0-vertex", TheDriver, + Diags); + validateTargetProfile("-Ths_6_1", "dxil--shadermodel6.1-hull", TheDriver, + Diags); + validateTargetProfile("-Tds_6_2", "dxil--shadermodel6.2-domain", TheDriver, + Diags); + validateTargetProfile("-Tds_6_2", "dxil--shadermodel6.2-domain", TheDriver, + Diags); + validateTargetProfile("-Tgs_6_3", "dxil--shadermodel6.3-geometry", TheDriver, + Diags); + validateTargetProfile("-Tps_6_4", "dxil--shadermodel6.4-pixel", TheDriver, + Diags); + validateTargetProfile("-Tcs_6_5", "dxil--shadermodel6.5-compute", TheDriver, + Diags); + validateTargetProfile("-Tms_6_6", "dxil--shadermodel6.6-mesh", TheDriver, + Diags); + validateTargetProfile("-Tas_6_7", "dxil--shadermodel6.7-amplification", + TheDriver, Diags); + validateTargetProfile("-Tlib_6_x", "dxil--shadermodel6.15-library", TheDriver, + Diags); // Invalid tests. - Args = TheDriver.ParseArgStrings({"-Tpss_6_1"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "unknown-unknown-shadermodel"); - EXPECT_EQ(Diags.getNumErrors(), 1u); - EXPECT_STREQ(DiagConsumer->Errors.back().c_str(), - "invalid profile : pss_6_1"); - Diags.Clear(); - DiagConsumer->clear(); + validateTargetProfile("-Tpss_6_1", "invalid profile : pss_6_1", TheDriver, + Diags, DiagConsumer, 1); - Args = TheDriver.ParseArgStrings({"-Tps_6_x"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "unknown-unknown-shadermodel"); - EXPECT_EQ(Diags.getNumErrors(), 2u); - EXPECT_STREQ(DiagConsumer->Errors.back().c_str(), "invalid profile : ps_6_x"); - Diags.Clear(); - DiagConsumer->clear(); - - Args = TheDriver.ParseArgStrings({"-Tlib_6_1"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "unknown-unknown-shadermodel"); - EXPECT_EQ(Diags.getNumErrors(), 3u); - EXPECT_STREQ(DiagConsumer->Errors.back().c_str(), - "invalid profile : lib_6_1"); - Diags.Clear(); - DiagConsumer->clear(); - - Args = TheDriver.ParseArgStrings({"-Tfoo"}, false, ContainsError); - EXPECT_FALSE(ContainsError); - Triple = TC.ComputeEffectiveClangTriple(Args); - EXPECT_STREQ(Triple.c_str(), "unknown-unknown-shadermodel"); - EXPECT_EQ(Diags.getNumErrors(), 4u); - EXPECT_STREQ(DiagConsumer->Errors.back().c_str(), "invalid profile : foo"); - Diags.Clear(); - DiagConsumer->clear(); + validateTargetProfile("-Tps_6_x", "invalid profile : ps_6_x", TheDriver, + Diags, DiagConsumer, 2); + validateTargetProfile("-Tlib_6_1", "invalid profile : lib_6_1", TheDriver, + Diags, DiagConsumer, 3); + validateTargetProfile("-Tfoo", "invalid profile : foo", TheDriver, Diags, + DiagConsumer, 4); + validateTargetProfile("", "target profile option (-T) is missing", TheDriver, + Diags, DiagConsumer, 5); } TEST(DxcModeTest, ValidatorVersionValidation) { @@ -520,8 +471,8 @@ TEST(DxcModeTest, ValidatorVersionValidation) { IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions(); DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer); Driver TheDriver("/bin/clang", "", Diags, "", InMemoryFileSystem); - std::unique_ptr C( - TheDriver.BuildCompilation({"clang", "--driver-mode=dxc", "foo.hlsl"})); + std::unique_ptr C(TheDriver.BuildCompilation( + {"clang", "--driver-mode=dxc", "-Tlib_6_7", "foo.hlsl"})); EXPECT_TRUE(C); EXPECT_TRUE(!C->containsError());