Revert "[DirectX] Fix DXIL part header version encoding" #91791
Merged
Conversation
This file contains hidden or 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
This reverts commit 195d8ac.
|
@llvm/pr-subscribers-backend-directx @llvm/pr-subscribers-llvm-binary-utilities Author: Justin Bogner (bogner) ChangesTest failures on big endian bots after this change. Reverts llvm/llvm-project#91506 Full diff: https://github.com/llvm/llvm-project/pull/91791.diff 6 Files Affected:
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
|
python3kgae
added a commit
that referenced
this pull request
May 13, 2024
)" This reverts commit d655054.
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.
Test failures on big endian bots after this change.
Reverts #91506