[llvm] [DirectX] Replace bitfield version in ProgramHeader. (PR #91797)
Xiang Li via llvm-commits
llvm-commits at lists.llvm.org
Fri May 10 12:40:04 PDT 2024
https://github.com/python3kgae created https://github.com/llvm/llvm-project/pull/91797
Avoid using bitfield in dxbc::ProgramHeader.
It could potentially be read incorrectly on any host depending on the compiler.
>From [C++17's [class.bit]](https://timsong-cpp.github.io/cppwp/n4659/class.bit#1)
> Bit-fields are packed into some addressable allocation unit. [ Note: Bit-fields straddle allocation units on some machines and not on others. Bit-fields are assigned right-to-left on some machines, left-to-right on others. — end note ]
For #91793
>From d32f8d40b3ed929870dc1c7709e80774e675ea28 Mon Sep 17 00:00:00 2001
From: Xiang Li <python3kgae at outlook.com>
Date: Fri, 10 May 2024 15:36:04 -0400
Subject: [PATCH] [DirectX] Replace bitfield version in ProgramHeader.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Avoid use bitfield in dxbc::ProgramHeader.
It could potentially be read incorrectly on any host depending on the compiler.
>From [C++17's [class.bit]](https://timsong-cpp.github.io/cppwp/n4659/class.bit#1)
> Bit-fields are packed into some addressable allocation unit. [ Note: Bit-fields straddle allocation units on some machines and not on others. Bit-fields are assigned right-to-left on some machines, left-to-right on others. — end note ]
For #91793
---
llvm/include/llvm/BinaryFormat/DXContainer.h | 8 +++-
llvm/lib/MC/MCDXContainerWriter.cpp | 8 ++--
llvm/lib/ObjectYAML/DXContainerEmitter.cpp | 4 +-
llvm/unittests/Object/DXContainerTest.cpp | 43 ++++++++++++++++++++
4 files changed, 56 insertions(+), 7 deletions(-)
diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h
index e8d03f806715f..847d8103e6811 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainer.h
+++ b/llvm/include/llvm/BinaryFormat/DXContainer.h
@@ -119,8 +119,7 @@ struct BitcodeHeader {
};
struct ProgramHeader {
- uint8_t MinorVersion : 4;
- uint8_t MajorVersion : 4;
+ uint8_t Version;
uint8_t Unused;
uint16_t ShaderKind;
uint32_t Size; // Size in uint32_t words including this header.
@@ -131,6 +130,11 @@ struct ProgramHeader {
sys::swapByteOrder(Size);
Bitcode.swapBytes();
}
+ uint8_t getMajorVersion() { return Version >> 4; }
+ uint8_t getMinorVersion() { return Version & 0xF; }
+ static uint8_t getVersion(uint8_t Major, uint8_t Minor) {
+ return (Major << 4) | Minor;
+ }
};
static_assert(sizeof(ProgramHeader) == 24, "ProgramHeader Size incorrect!");
diff --git a/llvm/lib/MC/MCDXContainerWriter.cpp b/llvm/lib/MC/MCDXContainerWriter.cpp
index 0580dc7e42826..ff64c6e538ac6 100644
--- a/llvm/lib/MC/MCDXContainerWriter.cpp
+++ b/llvm/lib/MC/MCDXContainerWriter.cpp
@@ -117,9 +117,11 @@ uint64_t DXContainerObjectWriter::writeObject(MCAssembler &Asm,
const Triple &TT = Asm.getContext().getTargetTriple();
VersionTuple Version = TT.getOSVersion();
- Header.MajorVersion = static_cast<uint8_t>(Version.getMajor());
- if (Version.getMinor())
- Header.MinorVersion = static_cast<uint8_t>(*Version.getMinor());
+ uint8_t MajorVersion = static_cast<uint8_t>(Version.getMajor());
+ uint8_t MinorVersion =
+ static_cast<uint8_t>(Version.getMinor().value_or(0));
+ Header.Version =
+ dxbc::ProgramHeader::getVersion(MajorVersion, MinorVersion);
if (TT.hasEnvironment())
Header.ShaderKind =
static_cast<uint16_t>(TT.getEnvironment() - Triple::Pixel);
diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
index f3a518df31750..175f1a12f9314 100644
--- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
@@ -131,8 +131,8 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
if (!P.Program)
continue;
dxbc::ProgramHeader Header;
- Header.MajorVersion = P.Program->MajorVersion;
- Header.MinorVersion = P.Program->MinorVersion;
+ Header.Version = dxbc::ProgramHeader::getVersion(P.Program->MajorVersion,
+ P.Program->MinorVersion);
Header.Unused = 0;
Header.ShaderKind = P.Program->ShaderKind;
memcpy(Header.Bitcode.Magic, "DXIL", 4);
diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index da640225617d4..115de47a3cefd 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -174,6 +174,49 @@ TEST(DXCFile, ParseEmptyParts) {
}
}
+// This test verify DXIL part 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.getMajorVersion(), 6u);
+ EXPECT_EQ(Header.getMinorVersion(), 5u);
+ EXPECT_EQ(Header.ShaderKind, 5u);
+ EXPECT_EQ(Header.Size, 8u);
+}
+
static Expected<DXContainer>
generateDXContainer(StringRef Yaml, SmallVectorImpl<char> &BinaryData) {
DXContainerYAML::Object Obj;
More information about the llvm-commits
mailing list