Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[DirectX][DXIL] Set DXIL Version in DXIL target triple based on shader model version" #91290

Merged
merged 1 commit into from
May 7, 2024

Conversation

bharadwajy
Copy link
Contributor

Reverts #90809

Need to investigate ASAN failures.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support llvm:ir labels May 7, 2024
@bharadwajy bharadwajy requested a review from bogner May 7, 2024 02:17
@llvmbot
Copy link

llvmbot commented May 7, 2024

@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-hlsl

Author: S. Bharadwaj Yadavalli (bharadwajy)

Changes

Reverts llvm/llvm-project#90809

Need to investigate ASAN failures.


Full diff: https://github.com/llvm/llvm-project/pull/91290.diff

10 Files Affected:

  • (modified) clang/lib/Basic/Targets.cpp (+1-1)
  • (modified) clang/lib/Driver/ToolChains/HLSL.cpp (+2-42)
  • (modified) clang/test/CodeGenHLSL/basic-target.c (+1-1)
  • (modified) clang/test/Driver/dxc_dxv_path.hlsl (+3-3)
  • (modified) clang/test/Options/enable_16bit_types_validation.hlsl (+2-2)
  • (modified) clang/unittests/Driver/DXCModeTest.cpp (+10-12)
  • (modified) llvm/include/llvm/TargetParser/Triple.h (-1)
  • (modified) llvm/lib/IR/Verifier.cpp (+2-2)
  • (modified) llvm/lib/TargetParser/Triple.cpp (-68)
  • (modified) llvm/unittests/TargetParser/TripleTest.cpp (-16)
diff --git a/clang/lib/Basic/Targets.cpp b/clang/lib/Basic/Targets.cpp
index dc1792b3471e6c..e3283510c6aac7 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(llvm::Triple::normalize(Opts->Triple));
+  llvm::Triple Triple(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 8286e3be21803f..558e4db46f8182 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -98,49 +98,9 @@ std::optional<std::string> tryParseProfile(StringRef Profile) {
   else if (llvm::getAsUnsignedInteger(Parts[2], 0, Minor))
     return std::nullopt;
 
-  // 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
+  // dxil-unknown-shadermodel-hull
   llvm::Triple T;
-  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.setArch(Triple::ArchType::dxil);
   T.setOSName(Triple::getOSTypeName(Triple::OSType::ShaderModel).str() +
               VersionTuple(Major, Minor).getAsString());
   T.setEnvironment(Kind);
diff --git a/clang/test/CodeGenHLSL/basic-target.c b/clang/test/CodeGenHLSL/basic-target.c
index b97ebf90a7a107..8db711c3f2a5b1 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 = "dxilv1.0-pc-shadermodel6.0-{{[a-z]+}}"
+// CHECK: target triple = "dxil-pc-shadermodel6.0-{{[a-z]+}}"
diff --git a/clang/test/Driver/dxc_dxv_path.hlsl b/clang/test/Driver/dxc_dxv_path.hlsl
index 4845de11d5b00b..3d8e90d0d91975 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" "dxilv1.3-unknown-shadermodel6.3-library"
+// 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: "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"]
+// BINDINGS: "dxil-unknown-shadermodel6.3-library" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[DXC:.+]].dxo"
+// BINDINGS-NEXT: "dxil-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 bcb217e8982e14..89fe26790c52be 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: "dxilv1.4-unknown-shadermodel6.4-library"
+// valid_2021: "dxil-unknown-shadermodel6.4-library"
 // valid_2021-SAME: "-std=hlsl2021"
 // valid_2021-SAME: "-fnative-half-type"
 
-// valid_2018: "dxilv1.4-unknown-shadermodel6.4-library"
+// valid_2018: "dxil-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 416723d498a240..b3767c042edb23 100644
--- a/clang/unittests/Driver/DXCModeTest.cpp
+++ b/clang/unittests/Driver/DXCModeTest.cpp
@@ -68,27 +68,25 @@ TEST(DxcModeTest, TargetProfileValidation) {
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer);
 
-  validateTargetProfile("-Tvs_6_0", "dxilv1.0--shadermodel6.0-vertex",
+  validateTargetProfile("-Tvs_6_0", "dxil--shadermodel6.0-vertex",
                         InMemoryFileSystem, Diags);
-  validateTargetProfile("-Ths_6_1", "dxilv1.1--shadermodel6.1-hull",
+  validateTargetProfile("-Ths_6_1", "dxil--shadermodel6.1-hull",
                         InMemoryFileSystem, Diags);
-  validateTargetProfile("-Tds_6_2", "dxilv1.2--shadermodel6.2-domain",
+  validateTargetProfile("-Tds_6_2", "dxil--shadermodel6.2-domain",
                         InMemoryFileSystem, Diags);
-  validateTargetProfile("-Tds_6_2", "dxilv1.2--shadermodel6.2-domain",
+  validateTargetProfile("-Tds_6_2", "dxil--shadermodel6.2-domain",
                         InMemoryFileSystem, Diags);
-  validateTargetProfile("-Tgs_6_3", "dxilv1.3--shadermodel6.3-geometry",
+  validateTargetProfile("-Tgs_6_3", "dxil--shadermodel6.3-geometry",
                         InMemoryFileSystem, Diags);
-  validateTargetProfile("-Tps_6_4", "dxilv1.4--shadermodel6.4-pixel",
+  validateTargetProfile("-Tps_6_4", "dxil--shadermodel6.4-pixel",
                         InMemoryFileSystem, Diags);
-  validateTargetProfile("-Tcs_6_5", "dxilv1.5--shadermodel6.5-compute",
+  validateTargetProfile("-Tcs_6_5", "dxil--shadermodel6.5-compute",
                         InMemoryFileSystem, Diags);
-  validateTargetProfile("-Tms_6_6", "dxilv1.6--shadermodel6.6-mesh",
+  validateTargetProfile("-Tms_6_6", "dxil--shadermodel6.6-mesh",
                         InMemoryFileSystem, Diags);
-  validateTargetProfile("-Tas_6_7", "dxilv1.7--shadermodel6.7-amplification",
+  validateTargetProfile("-Tas_6_7", "dxil--shadermodel6.7-amplification",
                         InMemoryFileSystem, Diags);
-  validateTargetProfile("-Tcs_6_8", "dxilv1.8--shadermodel6.8-compute",
-                        InMemoryFileSystem, Diags);
-  validateTargetProfile("-Tlib_6_x", "dxilv1.8--shadermodel6.15-library",
+  validateTargetProfile("-Tlib_6_x", "dxil--shadermodel6.15-library",
                         InMemoryFileSystem, Diags);
 
   // Invalid tests.
diff --git a/llvm/include/llvm/TargetParser/Triple.h b/llvm/include/llvm/TargetParser/Triple.h
index 4e357cddcf2a4b..7da30e6cf96f6c 100644
--- a/llvm/include/llvm/TargetParser/Triple.h
+++ b/llvm/include/llvm/TargetParser/Triple.h
@@ -176,7 +176,6 @@ class Triple {
     DXILSubArch_v1_6,
     DXILSubArch_v1_7,
     DXILSubArch_v1_8,
-    LatestDXILSubArch = DXILSubArch_v1_8,
   };
   enum VendorType {
     UnknownVendor,
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index a463e672bee90f..41d3fce7eef776 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(Triple::normalize(M.getTargetTriple())),
-        DL(M.getDataLayout()), Context(M.getContext()) {}
+      : OS(OS), M(M), MST(&M), TT(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 a6989790a98eb1..f3f244c814e7ee 100644
--- a/llvm/lib/TargetParser/Triple.cpp
+++ b/llvm/lib/TargetParser/Triple.cpp
@@ -115,31 +115,6 @@ 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:
-      break;
-    }
-    break;
   default:
     break;
   }
@@ -1039,8 +1014,6 @@ Triple::Triple(const Twine &ArchStr, const Twine &VendorStr, const Twine &OSStr,
     ObjectFormat = getDefaultFormat(*this);
 }
 
-static VersionTuple parseVersionFromName(StringRef Name);
-
 std::string Triple::normalize(StringRef Str) {
   bool IsMinGW32 = false;
   bool IsCygwin = false;
@@ -1233,47 +1206,6 @@ 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") {
-    if (Components.size() > 4) {
-      Components.resize(4);
-    }
-    // Add DXIL version only if shadermodel is specified in the triple
-    if (OS == Triple::ShaderModel) {
-      VersionTuple Ver =
-          parseVersionFromName(Components[2].drop_front(strlen("shadermodel")));
-      // Default DXIL minor version when Shader Model version is anything other
-      // than 6.[0...8] or 6.x (which translates to latest current SM version)
-      // DXIL version corresponding to Shader Model version other than 6.x
-      // is 1.0
-      unsigned DXILMinor = 0;
-      const unsigned SMMajor = 6;
-      const unsigned LatestCurrentDXILMinor = 8;
-      if (!Ver.empty()) {
-        if (Ver.getMajor() == SMMajor) {
-          if (std::optional<unsigned> SMMinor = Ver.getMinor()) {
-            DXILMinor = *SMMinor;
-            // Ensure specified minor version is supported
-            if (DXILMinor > LatestCurrentDXILMinor) {
-              report_fatal_error("Unsupported Shader Model version", false);
-            }
-          }
-        }
-      } else {
-        // Special case: DXIL minor version is set to LatestCurrentDXILMinor for
-        // shadermodel6.x is
-        if (Components[2] == "shadermodel6.x") {
-          DXILMinor = LatestCurrentDXILMinor;
-        }
-      }
-      Components[0] =
-          Components[0].str().append("v1.").append(std::to_string(DXILMinor));
-    }
-  }
   // Stick the corrected components back together to form the normalized string.
   return join(Components, "-");
 }
diff --git a/llvm/unittests/TargetParser/TripleTest.cpp b/llvm/unittests/TargetParser/TripleTest.cpp
index 3112014d9efb67..b8f5fbd87407d3 100644
--- a/llvm/unittests/TargetParser/TripleTest.cpp
+++ b/llvm/unittests/TargetParser/TripleTest.cpp
@@ -2454,20 +2454,4 @@ TEST(TripleTest, isArmMClass) {
     EXPECT_TRUE(T.isArmMClass());
   }
 }
-
-TEST(TripleTest, DXILNormaizeWithVersion) {
-  EXPECT_EQ("dxilv1.0-unknown-shadermodel6.0",
-            Triple::normalize("dxilv1.0--shadermodel6.0"));
-  EXPECT_EQ("dxilv1.0-unknown-shadermodel6.0",
-            Triple::normalize("dxil--shadermodel6.0"));
-  EXPECT_EQ("dxilv1.1-unknown-shadermodel6.1-library",
-            Triple::normalize("dxil-shadermodel6.1-unknown-library"));
-  EXPECT_EQ("dxilv1.8-unknown-shadermodel6.x-unknown",
-            Triple::normalize("dxil-unknown-shadermodel6.x-unknown"));
-  EXPECT_EQ("dxilv1.8-unknown-shadermodel6.x-unknown",
-            Triple::normalize("dxil-unknown-shadermodel6.x-unknown"));
-  EXPECT_EQ("dxil-unknown-unknown-unknown", Triple::normalize("dxil---"));
-  EXPECT_EQ("dxilv1.0-pc-shadermodel5.0-compute",
-            Triple::normalize("dxil-shadermodel5.0-pc-compute"));
-}
 } // end anonymous namespace

@bharadwajy bharadwajy requested a review from llvm-beanz May 7, 2024 02:17
@bharadwajy bharadwajy merged commit 178ff39 into main May 7, 2024
9 of 10 checks passed
@bharadwajy bharadwajy deleted the revert-90809-dxil-version-target-triple branch May 7, 2024 02:21
bharadwajy added a commit to bharadwajy/llvm-project that referenced this pull request May 7, 2024
… on shader model version" (llvm#91290)"

This restores commit 178ff39.

An earlier commit provided a way to decouple DXIL version from Shader
Model version by representing the DXIL version as `SubArch` in the DXIL
Target Triple and adding corresponding 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:

 1. When compilation target profile is specified:
    For e.g., DXIL target triple `dxilv1.8-unknown-shader6.8-library` is
    constructed when `-T lib_6_8` is specified.
 2. When DXIL target triple without DXIL version is specified:
    For e.g., DXIL target triple `dxilv1.8-pc-shadermodel6.8-library` is
    constructed when `-mtriple=dxil-pc-shadermodel6.8-library` is specified.

    Updated relevant HLSL tests that check for target triple.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support llvm:ir
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants