[llvm] [DirectX] Fix DXIL part header version encoding (PR #91506)

Xiang Li via llvm-commits llvm-commits at lists.llvm.org
Thu May 9 05:28:34 PDT 2024


https://github.com/python3kgae updated https://github.com/llvm/llvm-project/pull/91506

>From 9cd6ecf2a639826ddfe6ed4424bf46b4337a5093 Mon Sep 17 00:00:00 2001
From: Xiang Li <python3kgae at outlook.com>
Date: Wed, 8 May 2024 13:10:24 -0400
Subject: [PATCH 1/3] [DirectX] Fix DXIL part header version encoding

Move MinorVersion be the lower 8 bit.
Set DXIL version in DXContainerObjectWriter::writeObject.

Also always normalize target triple in llc to get DXIL version from subArch.

Fixes #89952
---
 llvm/include/llvm/BinaryFormat/DXContainer.h |  2 +-
 llvm/lib/MC/MCDXContainerWriter.cpp          |  3 ++
 llvm/test/CodeGen/DirectX/embed-dxil.ll      |  4 +-
 llvm/tools/llc/llc.cpp                       |  4 +-
 llvm/unittests/Object/DXContainerTest.cpp    | 57 +++++++++++++++++---
 5 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h
index e8d03f806715f..e5fcda63910d4 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 MajorVersion; // DXIL version.
   uint8_t MinorVersion; // DXIL version.
+  uint8_t MajorVersion; // 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/lib/MC/MCDXContainerWriter.cpp b/llvm/lib/MC/MCDXContainerWriter.cpp
index 0580dc7e42826..015899278f379 100644
--- a/llvm/lib/MC/MCDXContainerWriter.cpp
+++ b/llvm/lib/MC/MCDXContainerWriter.cpp
@@ -127,6 +127,9 @@ 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/test/CodeGen/DirectX/embed-dxil.ll b/llvm/test/CodeGen/DirectX/embed-dxil.ll
index 306e5c385b5a3..9f4fb19d86fa0 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: [[#]]
-; DXC-NEXT:       DXILMinorVersion: [[#]]
+; DXC-NEXT:       DXILMajorVersion: 1
+; DXC-NEXT:       DXILMinorVersion: 5
 ; DXC-NEXT:       DXILSize:        [[#SIZE - 24]]
 ; DXC-NEXT:       DXIL:            [ 0x42, 0x43, 0xC0, 0xDE,
 ; DXC:      - Name:            SFI0
diff --git a/llvm/tools/llc/llc.cpp b/llvm/tools/llc/llc.cpp
index b292f70ba89de..5825e79156774 100644
--- a/llvm/tools/llc/llc.cpp
+++ b/llvm/tools/llc/llc.cpp
@@ -537,8 +537,8 @@ static int compileModule(char **argv, LLVMContext &Context) {
       // If we are supposed to override the target triple, do so now.
       std::string IRTargetTriple = DataLayoutTargetTriple.str();
       if (!TargetTriple.empty())
-        IRTargetTriple = Triple::normalize(TargetTriple);
-      TheTriple = Triple(IRTargetTriple);
+        IRTargetTriple = TargetTriple;
+      TheTriple = Triple(Triple::normalize(IRTargetTriple));
       if (TheTriple.getTriple().empty())
         TheTriple.setTriple(sys::getDefaultTargetTriple());
 
diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index da640225617d4..28df84578113d 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -126,6 +126,51 @@ 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,
@@ -240,8 +285,8 @@ TEST(DXCFile, PSVResourceIterators) {
       MinorVersion:    0
       ShaderKind:      14
       Size:            6
-      DXILMajorVersion: 0
-      DXILMinorVersion: 1
+      DXILMajorVersion: 1
+      DXILMinorVersion: 0
       DXILSize:        0
 ...
 )";
@@ -361,8 +406,8 @@ TEST(DXCFile, PSVResourceIterators) {
 //       MinorVersion:    0
 //       ShaderKind:      14
 //       Size:            6
-//       DXILMajorVersion: 0
-//       DXILMinorVersion: 1
+//       DXILMajorVersion: 1
+//       DXILMinorVersion: 0
 //       DXILSize:        0
 //   - Name:            PSV0
 //     Size:            36
@@ -491,8 +536,8 @@ TEST(DXCFile, MaliciousFiles) {
 //       MinorVersion:    0
 //       ShaderKind:      14
 //       Size:            6
-//       DXILMajorVersion: 0
-//       DXILMinorVersion: 1
+//       DXILMajorVersion: 1
+//       DXILMinorVersion: 0
 //       DXILSize:        0
 //   - Name:            PSV0
 //     Size:            100

>From c04c073ccd933ea643a15b5dffbfefa3b92c7dd3 Mon Sep 17 00:00:00 2001
From: Xiang Li <python3kgae at outlook.com>
Date: Wed, 8 May 2024 13:35:37 -0400
Subject: [PATCH 2/3] Fix clang-format

---
 llvm/unittests/Object/DXContainerTest.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index 28df84578113d..098da331ab568 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -130,7 +130,7 @@ TEST(DXCFile, ParseOverlappingParts) {
 // 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, 
+//   Hash:            [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
 //                      0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
 //   Version:
 //     Major:           1
@@ -522,7 +522,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

>From ade02d7ed522b2135359b7e128d0d6ffef9ae1a9 Mon Sep 17 00:00:00 2001
From: Xiang Li <python3kgae at outlook.com>
Date: Wed, 8 May 2024 16:48:56 -0400
Subject: [PATCH 3/3] Only normlize Triple when not empty.

---
 llvm/tools/llc/llc.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/llvm/tools/llc/llc.cpp b/llvm/tools/llc/llc.cpp
index 5825e79156774..26f041e461a03 100644
--- a/llvm/tools/llc/llc.cpp
+++ b/llvm/tools/llc/llc.cpp
@@ -538,7 +538,9 @@ static int compileModule(char **argv, LLVMContext &Context) {
       std::string IRTargetTriple = DataLayoutTargetTriple.str();
       if (!TargetTriple.empty())
         IRTargetTriple = TargetTriple;
-      TheTriple = Triple(Triple::normalize(IRTargetTriple));
+      if (!IRTargetTriple.empty())
+        IRTargetTriple = Triple::normalize(IRTargetTriple);
+      TheTriple = Triple(IRTargetTriple);
       if (TheTriple.getTriple().empty())
         TheTriple.setTriple(sys::getDefaultTargetTriple());
 



More information about the llvm-commits mailing list