[llvm] Revert "[DirectX] Fix DXIL part header version encoding" (PR #91791)

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Fri May 10 11:42:12 PDT 2024


https://github.com/bogner created https://github.com/llvm/llvm-project/pull/91791

Test failures on big endian bots after this change.

Reverts llvm/llvm-project#91506

>From f5cec3acf9d2664c2209f24cc8ea0c5f3ed7fec6 Mon Sep 17 00:00:00 2001
From: Justin Bogner <mail at justinbogner.com>
Date: Fri, 10 May 2024 12:41:18 -0600
Subject: [PATCH] Revert "[DirectX] Fix DXIL part header version encoding
 (#91506)"

This reverts commit 195d8ac26d91ca798733c3a5f58d67992d43503d.
---
 llvm/include/llvm/BinaryFormat/DXContainer.h |  2 +-
 llvm/include/llvm/TargetParser/Triple.h      |  2 +-
 llvm/lib/MC/MCDXContainerWriter.cpp          |  3 -
 llvm/lib/TargetParser/Triple.cpp             |  2 -
 llvm/test/CodeGen/DirectX/embed-dxil.ll      |  4 +-
 llvm/unittests/Object/DXContainerTest.cpp    | 59 +++-----------------
 6 files changed, 11 insertions(+), 61 deletions(-)

diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h
index e5fcda63910d4..e8d03f806715f 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainer.h
+++ b/llvm/include/llvm/BinaryFormat/DXContainer.h
@@ -103,8 +103,8 @@ struct PartHeader {
 
 struct BitcodeHeader {
   uint8_t Magic[4];     // ACSII "DXIL".
-  uint8_t MinorVersion; // DXIL version.
   uint8_t MajorVersion; // DXIL version.
+  uint8_t MinorVersion; // DXIL version.
   uint16_t Unused;
   uint32_t Offset; // Offset to LLVM bitcode (from start of header).
   uint32_t Size;   // Size of LLVM bitcode (in bytes).
diff --git a/llvm/include/llvm/TargetParser/Triple.h b/llvm/include/llvm/TargetParser/Triple.h
index b3bb354b38ff5..8f9d99816931a 100644
--- a/llvm/include/llvm/TargetParser/Triple.h
+++ b/llvm/include/llvm/TargetParser/Triple.h
@@ -429,7 +429,7 @@ class Triple {
   /// (SubArch).  This should only be called with Vulkan SPIR-V triples.
   VersionTuple getVulkanVersion() const;
 
-  /// Parse the DXIL version number from the OSVersion and DXIL version
+  /// Parse the DXIL version number from the DXIL version
   /// (SubArch).  This should only be called with DXIL triples.
   VersionTuple getDXILVersion() const;
 
diff --git a/llvm/lib/MC/MCDXContainerWriter.cpp b/llvm/lib/MC/MCDXContainerWriter.cpp
index 015899278f379..0580dc7e42826 100644
--- a/llvm/lib/MC/MCDXContainerWriter.cpp
+++ b/llvm/lib/MC/MCDXContainerWriter.cpp
@@ -127,9 +127,6 @@ uint64_t DXContainerObjectWriter::writeObject(MCAssembler &Asm,
       // The program header's size field is in 32-bit words.
       Header.Size = (SectionSize + sizeof(dxbc::ProgramHeader) + 3) / 4;
       memcpy(Header.Bitcode.Magic, "DXIL", 4);
-      VersionTuple DXILVersion = TT.getDXILVersion();
-      Header.Bitcode.MajorVersion = DXILVersion.getMajor();
-      Header.Bitcode.MinorVersion = DXILVersion.getMinor().value_or(0);
       Header.Bitcode.Offset = sizeof(dxbc::BitcodeHeader);
       Header.Bitcode.Size = SectionSize;
       if (sys::IsBigEndianHost)
diff --git a/llvm/lib/TargetParser/Triple.cpp b/llvm/lib/TargetParser/Triple.cpp
index 4fc1ff5aaa051..f8269a51dc0bb 100644
--- a/llvm/lib/TargetParser/Triple.cpp
+++ b/llvm/lib/TargetParser/Triple.cpp
@@ -1510,8 +1510,6 @@ VersionTuple Triple::getDXILVersion() const {
   if (getArch() != dxil || getOS() != ShaderModel)
     llvm_unreachable("invalid DXIL triple");
   StringRef Arch = getArchName();
-  if (getSubArch() == NoSubArch)
-    Arch = getDXILArchNameFromShaderModel(getOSName());
   Arch.consume_front("dxilv");
   VersionTuple DXILVersion = parseVersionFromName(Arch);
   // FIXME: validate DXIL version against Shader Model version.
diff --git a/llvm/test/CodeGen/DirectX/embed-dxil.ll b/llvm/test/CodeGen/DirectX/embed-dxil.ll
index 9f4fb19d86fa0..306e5c385b5a3 100644
--- a/llvm/test/CodeGen/DirectX/embed-dxil.ll
+++ b/llvm/test/CodeGen/DirectX/embed-dxil.ll
@@ -42,8 +42,8 @@ define i32 @add(i32 %a, i32 %b) {
 ; DXC-NEXT:       MinorVersion:    5
 ; DXC-NEXT:       ShaderKind:      6
 ; DXC-NEXT:       Size:            [[#div(SIZE,4)]]
-; DXC-NEXT:       DXILMajorVersion: 1
-; DXC-NEXT:       DXILMinorVersion: 5
+; DXC-NEXT:       DXILMajorVersion: [[#]]
+; DXC-NEXT:       DXILMinorVersion: [[#]]
 ; DXC-NEXT:       DXILSize:        [[#SIZE - 24]]
 ; DXC-NEXT:       DXIL:            [ 0x42, 0x43, 0xC0, 0xDE,
 ; DXC:      - Name:            SFI0
diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index 098da331ab568..da640225617d4 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -126,51 +126,6 @@ TEST(DXCFile, ParseOverlappingParts) {
           "Part offset for part 1 begins before the previous part ends"));
 }
 
-// This test verify DXILMajorVersion and DXILMinorVersion are correctly parsed.
-// This test is based on the binary output constructed from this yaml.
-// --- !dxcontainer
-// Header:
-//   Hash:            [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
-//                      0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
-//   Version:
-//     Major:           1
-//     Minor:           0
-//   PartCount:       1
-// Parts:
-//   - Name:            DXIL
-//     Size:            28
-//     Program:
-//       MajorVersion:    6
-//       MinorVersion:    5
-//       ShaderKind:      5
-//       Size:            8
-//       DXILMajorVersion: 1
-//       DXILMinorVersion: 5
-//       DXILSize:        4
-//       DXIL:            [ 0x42, 0x43, 0xC0, 0xDE, ]
-// ...
-TEST(DXCFile, ParseDXILPart) {
-  uint8_t Buffer[] = {
-      0x44, 0x58, 0x42, 0x43, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
-      0x48, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
-      0x44, 0x58, 0x49, 0x4c, 0x1c, 0x00, 0x00, 0x00, 0x65, 0x00, 0x05, 0x00,
-      0x08, 0x00, 0x00, 0x00, 0x44, 0x58, 0x49, 0x4c, 0x05, 0x01, 0x00, 0x00,
-      0x10, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x42, 0x43, 0xc0, 0xde};
-  DXContainer C =
-      llvm::cantFail(DXContainer::create(getMemoryBuffer<116>(Buffer)));
-  EXPECT_EQ(C.getHeader().PartCount, 1u);
-  const std::optional<object::DXContainer::DXILData> &DXIL = C.getDXIL();
-  EXPECT_TRUE(DXIL.has_value());
-  dxbc::ProgramHeader Header = DXIL->first;
-  EXPECT_EQ(Header.MajorVersion, 6u);
-  EXPECT_EQ(Header.MinorVersion, 5u);
-  EXPECT_EQ(Header.ShaderKind, 5u);
-  EXPECT_EQ(Header.Size, 8u);
-  EXPECT_EQ(Header.Bitcode.MajorVersion, 1u);
-  EXPECT_EQ(Header.Bitcode.MinorVersion, 5u);
-}
-
 TEST(DXCFile, ParseEmptyParts) {
   uint8_t Buffer[] = {
       0x44, 0x58, 0x42, 0x43, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
@@ -285,8 +240,8 @@ TEST(DXCFile, PSVResourceIterators) {
       MinorVersion:    0
       ShaderKind:      14
       Size:            6
-      DXILMajorVersion: 1
-      DXILMinorVersion: 0
+      DXILMajorVersion: 0
+      DXILMinorVersion: 1
       DXILSize:        0
 ...
 )";
@@ -406,8 +361,8 @@ TEST(DXCFile, PSVResourceIterators) {
 //       MinorVersion:    0
 //       ShaderKind:      14
 //       Size:            6
-//       DXILMajorVersion: 1
-//       DXILMinorVersion: 0
+//       DXILMajorVersion: 0
+//       DXILMinorVersion: 1
 //       DXILSize:        0
 //   - Name:            PSV0
 //     Size:            36
@@ -522,7 +477,7 @@ TEST(DXCFile, MaliciousFiles) {
 //
 // --- !dxcontainer
 // Header:
-//   Hash:            [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+//   Hash:            [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 
 //                      0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
 //   Version:
 //     Major:           1
@@ -536,8 +491,8 @@ TEST(DXCFile, MaliciousFiles) {
 //       MinorVersion:    0
 //       ShaderKind:      14
 //       Size:            6
-//       DXILMajorVersion: 1
-//       DXILMinorVersion: 0
+//       DXILMajorVersion: 0
+//       DXILMinorVersion: 1
 //       DXILSize:        0
 //   - Name:            PSV0
 //     Size:            100



More information about the llvm-commits mailing list