[DirectX][DXIL] Set DXIL Version in DXIL target triple based on shader model version #90809
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-hlsl Author: S. Bharadwaj Yadavalli (bharadwajy) ChangesAn earlier commit provided a way to decouple DXIL version from Shader Model version This change constructs DXIL target triple with DXIL version that is deduced
Note that this is an alternate / expanded implementation to that proposed in PR #89823. PR #89823 implements functionality (1) above. However, it requires any DXIL target This PR adds the functionality (2) to eliminate the above requirement to explicitly Update one MIR test to reflect use of normalized target triple. Validated that Clang ( Full diff: https://github.com/llvm/llvm-project/pull/90809.diff 13 Files Affected:
diff --git a/clang/lib/Basic/Targets.cpp b/clang/lib/Basic/Targets.cpp
index e3283510c6aac7..dc1792b3471e6c 100644
--- a/clang/lib/Basic/Targets.cpp
+++ b/clang/lib/Basic/Targets.cpp
@@ -760,7 +760,7 @@ using namespace clang::targets;
TargetInfo *
TargetInfo::CreateTargetInfo(DiagnosticsEngine &Diags,
const std::shared_ptr<TargetOptions> &Opts) {
- llvm::Triple Triple(Opts->Triple);
+ llvm::Triple Triple(llvm::Triple::normalize(Opts->Triple));
// Construct the target
std::unique_ptr<TargetInfo> Target = AllocateTarget(Triple, *Opts);
diff --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 1169b5d8c92dd6..c4c92613f44723 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -98,9 +98,49 @@ std::optional<std::string> tryParseProfile(StringRef Profile) {
else if (llvm::getAsUnsignedInteger(Parts[2], 0, Minor))
return std::nullopt;
- // dxil-unknown-shadermodel-hull
+ // Determine DXIL version using the minor version number of Shader
+ // Model version specified in target profile. Prior to decoupling DXIL version
+ // numbering from that of Shader Model DXIL version 1.Y corresponds to SM 6.Y.
+ // E.g., dxilv1.Y-unknown-shadermodelX.Y-hull
llvm::Triple T;
- T.setArch(Triple::ArchType::dxil);
+ Triple::SubArchType SubArch = llvm::Triple::NoSubArch;
+ switch (Minor) {
+ case 0:
+ SubArch = llvm::Triple::DXILSubArch_v1_0;
+ break;
+ case 1:
+ SubArch = llvm::Triple::DXILSubArch_v1_1;
+ break;
+ case 2:
+ SubArch = llvm::Triple::DXILSubArch_v1_2;
+ break;
+ case 3:
+ SubArch = llvm::Triple::DXILSubArch_v1_3;
+ break;
+ case 4:
+ SubArch = llvm::Triple::DXILSubArch_v1_4;
+ break;
+ case 5:
+ SubArch = llvm::Triple::DXILSubArch_v1_5;
+ break;
+ case 6:
+ SubArch = llvm::Triple::DXILSubArch_v1_6;
+ break;
+ case 7:
+ SubArch = llvm::Triple::DXILSubArch_v1_7;
+ break;
+ case 8:
+ SubArch = llvm::Triple::DXILSubArch_v1_8;
+ break;
+ case OfflineLibMinor:
+ // Always consider minor version x as the latest supported DXIL version
+ SubArch = llvm::Triple::LatestDXILSubArch;
+ break;
+ default:
+ // No DXIL Version corresponding to specified Shader Model version found
+ return std::nullopt;
+ }
+ T.setArch(Triple::ArchType::dxil, SubArch);
T.setOSName(Triple::getOSTypeName(Triple::OSType::ShaderModel).str() +
VersionTuple(Major, Minor).getAsString());
T.setEnvironment(Kind);
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 8312abc3603953..a2600174e02296 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -587,7 +587,8 @@ static bool FixupInvocation(CompilerInvocation &Invocation,
LangOpts.SpeculativeLoadHardening = CodeGenOpts.SpeculativeLoadHardening;
LangOpts.CurrentModule = LangOpts.ModuleName;
- llvm::Triple T(TargetOpts.Triple);
+ llvm::Triple T(llvm::Triple::normalize(TargetOpts.Triple));
+
llvm::Triple::ArchType Arch = T.getArch();
CodeGenOpts.CodeModel = TargetOpts.CodeModel;
@@ -4675,7 +4676,7 @@ bool CompilerInvocation::CreateFromArgsImpl(
// FIXME: We shouldn't have to pass the DashX option around here
InputKind DashX = Res.getFrontendOpts().DashX;
ParseTargetArgs(Res.getTargetOpts(), Args, Diags);
- llvm::Triple T(Res.getTargetOpts().Triple);
+ llvm::Triple T(llvm::Triple::normalize(Res.getTargetOpts().Triple));
ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Diags,
Res.getFileSystemOpts().WorkingDir);
ParseAPINotesArgs(Res.getAPINotesOpts(), Args, Diags);
@@ -4901,7 +4902,7 @@ std::string CompilerInvocation::getModuleHash() const {
void CompilerInvocationBase::generateCC1CommandLine(
ArgumentConsumer Consumer) const {
- llvm::Triple T(getTargetOpts().Triple);
+ llvm::Triple T(llvm::Triple::normalize(getTargetOpts().Triple));
GenerateFileSystemArgs(getFileSystemOpts(), Consumer);
GenerateMigratorArgs(getMigratorOpts(), Consumer);
diff --git a/clang/test/CodeGenHLSL/basic-target.c b/clang/test/CodeGenHLSL/basic-target.c
index 8db711c3f2a5b1..b97ebf90a7a107 100644
--- a/clang/test/CodeGenHLSL/basic-target.c
+++ b/clang/test/CodeGenHLSL/basic-target.c
@@ -7,4 +7,4 @@
// RUN: %clang -target dxil-pc-shadermodel6.0-geometry -S -emit-llvm -o - %s | FileCheck %s
// CHECK: target datalayout = "e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:32-f64:64-n8:16:32:64"
-// CHECK: target triple = "dxil-pc-shadermodel6.0-{{[a-z]+}}"
+// CHECK: target triple = "dxilv1.0-pc-shadermodel6.0-{{[a-z]+}}"
diff --git a/clang/test/Driver/dxc_dxv_path.hlsl b/clang/test/Driver/dxc_dxv_path.hlsl
index 3d8e90d0d91975..4845de11d5b00b 100644
--- a/clang/test/Driver/dxc_dxv_path.hlsl
+++ b/clang/test/Driver/dxc_dxv_path.hlsl
@@ -7,12 +7,12 @@
// 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:"-cc1"{{.*}}"-triple" "dxilv1.3-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"]
+// BINDINGS: "dxilv1.3-unknown-shadermodel6.3-library" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[DXC:.+]].dxo"
+// BINDINGS-NEXT: "dxilv1.3-unknown-shadermodel6.3-library" - "hlsl::Validator", inputs: ["[[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
diff --git a/clang/test/Options/enable_16bit_types_validation.hlsl b/clang/test/Options/enable_16bit_types_validation.hlsl
index 71d336f6f5039f..f75a30ad80384f 100644
--- a/clang/test/Options/enable_16bit_types_validation.hlsl
+++ b/clang/test/Options/enable_16bit_types_validation.hlsl
@@ -9,11 +9,11 @@
// HV_invalid_2017: error: '-enable-16bit-types' option requires target HLSL Version >= 2018 and shader model >= 6.2, but HLSL Version is 'hlsl2017' and shader model is '6.4'
// TP_invalid: error: '-enable-16bit-types' option requires target HLSL Version >= 2018 and shader model >= 6.2, but HLSL Version is 'hlsl2021' and shader model is '6.0'
-// valid_2021: "dxil-unknown-shadermodel6.4-library"
+// valid_2021: "dxilv1.4-unknown-shadermodel6.4-library"
// valid_2021-SAME: "-std=hlsl2021"
// valid_2021-SAME: "-fnative-half-type"
-// valid_2018: "dxil-unknown-shadermodel6.4-library"
+// valid_2018: "dxilv1.4-unknown-shadermodel6.4-library"
// valid_2018-SAME: "-std=hlsl2018"
// valid_2018-SAME: "-fnative-half-type"
diff --git a/clang/unittests/Driver/DXCModeTest.cpp b/clang/unittests/Driver/DXCModeTest.cpp
index b3767c042edb23..416723d498a240 100644
--- a/clang/unittests/Driver/DXCModeTest.cpp
+++ b/clang/unittests/Driver/DXCModeTest.cpp
@@ -68,25 +68,27 @@ TEST(DxcModeTest, TargetProfileValidation) {
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer);
- validateTargetProfile("-Tvs_6_0", "dxil--shadermodel6.0-vertex",
+ validateTargetProfile("-Tvs_6_0", "dxilv1.0--shadermodel6.0-vertex",
InMemoryFileSystem, Diags);
- validateTargetProfile("-Ths_6_1", "dxil--shadermodel6.1-hull",
+ validateTargetProfile("-Ths_6_1", "dxilv1.1--shadermodel6.1-hull",
InMemoryFileSystem, Diags);
- validateTargetProfile("-Tds_6_2", "dxil--shadermodel6.2-domain",
+ validateTargetProfile("-Tds_6_2", "dxilv1.2--shadermodel6.2-domain",
InMemoryFileSystem, Diags);
- validateTargetProfile("-Tds_6_2", "dxil--shadermodel6.2-domain",
+ validateTargetProfile("-Tds_6_2", "dxilv1.2--shadermodel6.2-domain",
InMemoryFileSystem, Diags);
- validateTargetProfile("-Tgs_6_3", "dxil--shadermodel6.3-geometry",
+ validateTargetProfile("-Tgs_6_3", "dxilv1.3--shadermodel6.3-geometry",
InMemoryFileSystem, Diags);
- validateTargetProfile("-Tps_6_4", "dxil--shadermodel6.4-pixel",
+ validateTargetProfile("-Tps_6_4", "dxilv1.4--shadermodel6.4-pixel",
InMemoryFileSystem, Diags);
- validateTargetProfile("-Tcs_6_5", "dxil--shadermodel6.5-compute",
+ validateTargetProfile("-Tcs_6_5", "dxilv1.5--shadermodel6.5-compute",
InMemoryFileSystem, Diags);
- validateTargetProfile("-Tms_6_6", "dxil--shadermodel6.6-mesh",
+ validateTargetProfile("-Tms_6_6", "dxilv1.6--shadermodel6.6-mesh",
InMemoryFileSystem, Diags);
- validateTargetProfile("-Tas_6_7", "dxil--shadermodel6.7-amplification",
+ validateTargetProfile("-Tas_6_7", "dxilv1.7--shadermodel6.7-amplification",
InMemoryFileSystem, Diags);
- validateTargetProfile("-Tlib_6_x", "dxil--shadermodel6.15-library",
+ validateTargetProfile("-Tcs_6_8", "dxilv1.8--shadermodel6.8-compute",
+ InMemoryFileSystem, Diags);
+ validateTargetProfile("-Tlib_6_x", "dxilv1.8--shadermodel6.15-library",
InMemoryFileSystem, Diags);
// Invalid tests.
diff --git a/llvm/include/llvm/TargetParser/Triple.h b/llvm/include/llvm/TargetParser/Triple.h
index 7da30e6cf96f6c..4e357cddcf2a4b 100644
--- a/llvm/include/llvm/TargetParser/Triple.h
+++ b/llvm/include/llvm/TargetParser/Triple.h
@@ -176,6 +176,7 @@ class Triple {
DXILSubArch_v1_6,
DXILSubArch_v1_7,
DXILSubArch_v1_8,
+ LatestDXILSubArch = DXILSubArch_v1_8,
};
enum VendorType {
UnknownVendor,
diff --git a/llvm/lib/CodeGen/CommandFlags.cpp b/llvm/lib/CodeGen/CommandFlags.cpp
index 14ac4b2102c2fa..da0a6e023ffd45 100644
--- a/llvm/lib/CodeGen/CommandFlags.cpp
+++ b/llvm/lib/CodeGen/CommandFlags.cpp
@@ -744,7 +744,7 @@ void codegen::setFunctionAttributes(StringRef CPU, StringRef Features,
Expected<std::unique_ptr<TargetMachine>>
codegen::createTargetMachineForTriple(StringRef TargetTriple,
CodeGenOptLevel OptLevel) {
- Triple TheTriple(TargetTriple);
+ Triple TheTriple(llvm::Triple::normalize(TargetTriple.str()));
std::string Error;
const auto *TheTarget =
TargetRegistry::lookupTarget(codegen::getMArch(), TheTriple, Error);
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 16ed167bd67195..be63591583c507 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -152,8 +152,8 @@ struct VerifierSupport {
bool TreatBrokenDebugInfoAsError = true;
explicit VerifierSupport(raw_ostream *OS, const Module &M)
- : OS(OS), M(M), MST(&M), TT(M.getTargetTriple()), DL(M.getDataLayout()),
- Context(M.getContext()) {}
+ : OS(OS), M(M), MST(&M), TT(Triple::normalize(M.getTargetTriple())),
+ DL(M.getDataLayout()), Context(M.getContext()) {}
private:
void Write(const Module *M) {
diff --git a/llvm/lib/TargetParser/Triple.cpp b/llvm/lib/TargetParser/Triple.cpp
index 2c5aee3dfb2f3e..1b1e8c9df3f3f3 100644
--- a/llvm/lib/TargetParser/Triple.cpp
+++ b/llvm/lib/TargetParser/Triple.cpp
@@ -115,6 +115,30 @@ StringRef Triple::getArchName(ArchType Kind, SubArchType SubArch) {
if (SubArch == AArch64SubArch_arm64e)
return "arm64e";
break;
+ case Triple::dxil:
+ switch (SubArch) {
+ case Triple::NoSubArch:
+ case Triple::DXILSubArch_v1_0:
+ return "dxilv1.0";
+ case Triple::DXILSubArch_v1_1:
+ return "dxilv1.1";
+ case Triple::DXILSubArch_v1_2:
+ return "dxilv1.2";
+ case Triple::DXILSubArch_v1_3:
+ return "dxilv1.3";
+ case Triple::DXILSubArch_v1_4:
+ return "dxilv1.4";
+ case Triple::DXILSubArch_v1_5:
+ return "dxilv1.5";
+ case Triple::DXILSubArch_v1_6:
+ return "dxilv1.6";
+ case Triple::DXILSubArch_v1_7:
+ return "dxilv1.7";
+ case Triple::DXILSubArch_v1_8:
+ return "dxilv1.8";
+ default:
+ return "";
+ }
default:
break;
}
@@ -1200,6 +1224,27 @@ std::string Triple::normalize(StringRef Str) {
}
}
+ // Normalize DXIL triple if it does not include DXIL version number.
+ // Determine DXIL version number using the minor version number of Shader
+ // Model version specified in target triple, if any. Prior to decoupling DXIL
+ // version numbering from that of Shader Model DXIL version 1.Y corresponds to
+ // SM 6.Y. E.g., dxilv1.Y-unknown-shadermodelX.Y-hull
+ if (Components[0] == "dxil") {
+ std::string DXILVerStr{"dxilv1."};
+ if (Components.size() > 2) {
+ // OS component specified
+ if (Components[2].starts_with("shadermodel6.")) {
+ Components[0] = DXILVerStr.append(
+ Components[2].drop_front(strlen("shadermodel6.")));
+ } else if (Components[2].starts_with("shadermodel")) {
+ // If shader model specified is other than 6.x, set DXIL Version to 1.0
+ Components[0] = DXILVerStr.append("0");
+ }
+ }
+ // DXIL version is not set for a non-specified OS string or one that does
+ // not starts with shadermodel.
+ }
+
// Stick the corrected components back together to form the normalized string.
return join(Components, "-");
}
diff --git a/llvm/test/tools/llvm-reduce/mir/infer-triple-unknown-target.mir b/llvm/test/tools/llvm-reduce/mir/infer-triple-unknown-target.mir
index 251dd90f136c2f..0d77ca012a341b 100644
--- a/llvm/test/tools/llvm-reduce/mir/infer-triple-unknown-target.mir
+++ b/llvm/test/tools/llvm-reduce/mir/infer-triple-unknown-target.mir
@@ -1,6 +1,6 @@
# RUN: not llvm-reduce --test FileCheck %s -o /dev/null 2>&1 | FileCheck %s
-# CHECK: error: unable to get target for 'omgwtfbbqcpu64--', see --version and --triple.
+# CHECK: error: unable to get target for 'omgwtfbbqcpu64-unknown-unknown', see --version and --triple.
--- |
target triple = "omgwtfbbqcpu64--"
diff --git a/llvm/tools/opt/optdriver.cpp b/llvm/tools/opt/optdriver.cpp
index 948148bb80498c..2eaac7f93fe5ea 100644
--- a/llvm/tools/opt/optdriver.cpp
+++ b/llvm/tools/opt/optdriver.cpp
@@ -630,7 +630,7 @@ extern "C" int optMain(
}
}
- Triple ModuleTriple(M->getTargetTriple());
+ Triple ModuleTriple(Triple::normalize(M->getTargetTriple()));
std::string CPUStr, FeaturesStr;
std::unique_ptr<TargetMachine> TM;
if (ModuleTriple.getArch()) {
|
bob80905
reviewed
May 2, 2024
llvm/lib/TargetParser/Triple.cpp
Outdated
| case Triple::DXILSubArch_v1_8: | ||
| return "dxilv1.8"; | ||
| default: | ||
| return ""; |
There was a problem hiding this comment.
Would we want to llvm_unreachable or otherwise fail here, or does the caller handle this case well?
There was a problem hiding this comment.
Would we want to llvm_unreachable or otherwise fail here, or does the caller handle this case well?
The default case would be true for a call such as getArchName(Triple::dxil, Triple::<NonDXILSubArch>),
where NonDXILSubArch is something other than DXILSubArch_v1_[0..8] (e.g., MipsSubArch_r6).
I considered 3 options to handle this case:
- Report failure / crash with a message indicating incorrect DXIL
SubArch: not chosen as such behavior is not consistent with otherSuArchs as pointed out here in the feedback for PR [DirectX] Set DXIL Version using shader model version in compilation target profile #89823. - Fall through to return the value of
getArchTypeName(Kind)whereKindisTriple::dxilto return the string "dxil": not chosen since "dxil" does not represent theArchTypeNamefor theArch/SubArchcombination ofTriple::dxil/Triple::<NonDXILSubArch>(e.g.,Triple::dxil//Triple::MipsSubArch_r6) and returning "dxil" would indicate that it is a validArchTypeName. - Return null string (""): chosen to indicate that there is no valid
ArchTypeNamefor theArch/SubArchcombination ofTriple::dxil/Triple::<NonDXILSubArch>(e.g.,Triple::dxil/Triple::MipsSubArch_r6). The consumer of the Triple checks for validity of Arch to issue an error for a non-viable empty architecture name as I think it is a cleaner option compared to either reporting a failure or asserting here.
There was a problem hiding this comment.
I would argue that returning an empty string here is just as inconsistent as erroring out/crashing. We should probably just do (2) even though it's obviously ridiculous, since for other architectures if you provide an invalid subarch they are indeed just as ridiculous.
If we want to shore this up so that invalid sub architectures do something more sensible we should do it for all targets and in a separate change.
There was a problem hiding this comment.
I would argue that returning an empty string here is just as inconsistent as erroring out/crashing. We should probably just do (2) even though it's obviously ridiculous, since for other architectures if you provide an invalid subarch they are indeed just as ridiculous.
OK. Pushed a change that uses option (2).
python3kgae
reviewed
May 2, 2024
damyanp
approved these changes
May 2, 2024
python3kgae
reviewed
May 2, 2024
bob80905
approved these changes
May 3, 2024
llvm-beanz
reviewed
May 3, 2024
llvm-beanz
reviewed
May 3, 2024
There was a problem hiding this comment.
Given that @bogner had concerns about the other approach I think we should get him to review this before moving forward.
That said, other than some missing unit test coverage I think this approach looks fine.
bogner
reviewed
May 3, 2024
| // If shader model specified is other than 6.x, set DXIL Version to 1.0 | ||
| Components[0] = DXILVerStr.append("0"); | ||
| } | ||
| } |
There was a problem hiding this comment.
This doesn't look right. We've already parsed Arch and OS above, so I would expect this logic to look something like:
if (Arch == Triple::DXIL && SubArch == Triple::NoSubArch) {
VersionTuple Ver = OS != Triple::UnknownOS ? parseVersionFromName(Components[2]) : VersionTuple();
if (Ver)
Components[0] = Components[0].append("1.").append(Ver.Minor);
else (Ver)
// default case...
}There was a problem hiding this comment.
The function normalize does not parse for SubArch. So, SubArch is not available for checking.
An input DXIL triple string Str with or without version (e.g., dxil-pc-shadermodel6.3 or dxilv1.3-pc-shadermodel6.3 will set Arch to Triple::dxil. Implicit deduction and insertion of DXIL version should be done only if Component[0] == "dxil" and not otherwise. Hence the string equivalence check for "dxil" and not for Arch == Triple::dxil.
However, made code changes to leverage Arch and OS values already parsed as suggested. A couple of additional sanity checks are also added.
bogner
approved these changes
May 6, 2024
There was a problem hiding this comment.
Other than my comment here about what to do in getArchName this looks reasonable.
The string handling in normalize feels a bit unwieldy, but I don't have any obvious thoughts for how to simplify it.
llvm/lib/CodeGen/CommandFlags.cpp
Outdated
| @@ -744,7 +744,7 @@ void codegen::setFunctionAttributes(StringRef CPU, StringRef Features, | |||
| Expected<std::unique_ptr<TargetMachine>> | |||
| codegen::createTargetMachineForTriple(StringRef TargetTriple, | |||
| CodeGenOptLevel OptLevel) { | |||
| Triple TheTriple(TargetTriple); | |||
| Triple TheTriple(TargetTriple.str()); | |||
There was a problem hiding this comment.
This looks redundant - doesn't Triple's constructor do the conversion to std::string from Twine internally?
There was a problem hiding this comment.
This looks redundant - doesn't Triple's constructor do the conversion to
std::stringfrom Twine internally?
Thanks for flagging my incomplete restoration. Fixed.
llvm-beanz
approved these changes
May 6, 2024
a) specified as compilation target profile or b) specified as target triple string Update relevant HLSL tests that check for target triple.
Add unit tests for DXIL triple normalization
…y normalized as pointed out in PR feedback.
As a result any dxil triple specified in testing tools that invoke this function is treated unmodified and intentional. Hence it is not modified to include an implicit DXIL version - per PR feedback. Restored change to MIR test file.
thereby using any dxil triple as specified.
python3kgae
reviewed
May 6, 2024
8 tasks
bharadwajy
force-pushed
the
dxil-version-target-triple
branch
from
72be873
to
e739a33
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
This was referenced May 6, 2024
llvm-beanz
reviewed
May 7, 2024
| } | ||
| } | ||
| Components[0] = | ||
| Components[0].str().append("v1.").append(std::to_string(DXILMinor)); |
There was a problem hiding this comment.
@bharadwajy this is your asan failure. You’re setting a stringref to a locally modified string temporary.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An earlier commit provided a way to decouple DXIL version from Shader Model version
by representing the DXIL version as
SubArchin the DXIL Target Triple and addingcorresponding valid DXIL Arch types.
This change constructs DXIL target triple with DXIL version that is deduced
from Shader Model version specified in the following scenarios:
For e.g., DXIL target triple
dxilv1.8-unknown-shader6.8-libraryis constructedwhen
-T lib_6_8is specified.For e.g., DXIL target triple
dxilv1.8-pc-shadermodel6.8-libraryis constructedwhen
-mtriple=dxil-pc-shadermodel6.8-libraryis specified.Note that this is an alternate / expanded implementation to that proposed in PR #89823.
PR #89823 implements functionality (1) above. However, it requires any DXIL target
triple to explicitly include DXIL version anywhere DXIL target triple is expected (e.g.,
dxilv1.8-pc-shadermodel6.8-library) while a triple without DXIL version(e.g.,
dxil-pc-shadermodel6.8-library) is considered invalid. This functionality isimplemented as a change to
tryParseProfile()and is included in this PR.This PR adds the functionality (2) to eliminate the above requirement to explicitly
specify DXIL version in the target triple thereby considering a triple without DXIL
version ( e.g.,
dxil-pc-shadermodel6.8-library) to be valid. A triple with DXIL versionis inferred based on the shader mode version specified. If no shader model version is
specified, DXIL version is defaulted to 1.0. This functionality is implemented in the
"Special case logic ..." section of
Triple::normalize().Updated relevant HLSL tests that check for target triple.
Validated that Clang (
check-clang) and LLVM (check-llvm) regression tests pass.