Skip to content

Commit f42117c

Browse files
authored
[DirectX] Replace bitfield version in ProgramHeader. (#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
1 parent 0869204 commit f42117c

File tree

5 files changed

+58
-9
lines changed

5 files changed

+58
-9
lines changed

llvm/include/llvm/BinaryFormat/DXContainer.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,7 @@ struct BitcodeHeader {
119119
};
120120

121121
struct ProgramHeader {
122-
uint8_t MinorVersion : 4;
123-
uint8_t MajorVersion : 4;
122+
uint8_t Version;
124123
uint8_t Unused;
125124
uint16_t ShaderKind;
126125
uint32_t Size; // Size in uint32_t words including this header.
@@ -131,6 +130,11 @@ struct ProgramHeader {
131130
sys::swapByteOrder(Size);
132131
Bitcode.swapBytes();
133132
}
133+
uint8_t getMajorVersion() { return Version >> 4; }
134+
uint8_t getMinorVersion() { return Version & 0xF; }
135+
static uint8_t getVersion(uint8_t Major, uint8_t Minor) {
136+
return (Major << 4) | Minor;
137+
}
134138
};
135139

136140
static_assert(sizeof(ProgramHeader) == 24, "ProgramHeader Size incorrect!");

llvm/lib/MC/MCDXContainerWriter.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,11 @@ uint64_t DXContainerObjectWriter::writeObject(MCAssembler &Asm,
117117

118118
const Triple &TT = Asm.getContext().getTargetTriple();
119119
VersionTuple Version = TT.getOSVersion();
120-
Header.MajorVersion = static_cast<uint8_t>(Version.getMajor());
121-
if (Version.getMinor())
122-
Header.MinorVersion = static_cast<uint8_t>(*Version.getMinor());
120+
uint8_t MajorVersion = static_cast<uint8_t>(Version.getMajor());
121+
uint8_t MinorVersion =
122+
static_cast<uint8_t>(Version.getMinor().value_or(0));
123+
Header.Version =
124+
dxbc::ProgramHeader::getVersion(MajorVersion, MinorVersion);
123125
if (TT.hasEnvironment())
124126
Header.ShaderKind =
125127
static_cast<uint16_t>(TT.getEnvironment() - Triple::Pixel);

llvm/lib/ObjectYAML/DXContainerEmitter.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
131131
if (!P.Program)
132132
continue;
133133
dxbc::ProgramHeader Header;
134-
Header.MajorVersion = P.Program->MajorVersion;
135-
Header.MinorVersion = P.Program->MinorVersion;
134+
Header.Version = dxbc::ProgramHeader::getVersion(P.Program->MajorVersion,
135+
P.Program->MinorVersion);
136136
Header.Unused = 0;
137137
Header.ShaderKind = P.Program->ShaderKind;
138138
memcpy(Header.Bitcode.Magic, "DXIL", 4);

llvm/tools/obj2yaml/dxcontainer2yaml.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ dumpDXContainer(MemoryBufferRef Source) {
5858
assert(DXIL && "Since we are iterating and found a DXIL part, "
5959
"this should never not have a value");
6060
NewPart.Program = DXContainerYAML::DXILProgram{
61-
DXIL->first.MajorVersion,
62-
DXIL->first.MinorVersion,
61+
DXIL->first.getMajorVersion(),
62+
DXIL->first.getMinorVersion(),
6363
DXIL->first.ShaderKind,
6464
DXIL->first.Size,
6565
DXIL->first.Bitcode.MajorVersion,

llvm/unittests/Object/DXContainerTest.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,49 @@ TEST(DXCFile, ParseEmptyParts) {
174174
}
175175
}
176176

177+
// This test verify DXIL part are correctly parsed.
178+
// This test is based on the binary output constructed from this yaml.
179+
// --- !dxcontainer
180+
// Header:
181+
// Hash: [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
182+
// 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
183+
// Version:
184+
// Major: 1
185+
// Minor: 0
186+
// PartCount: 1
187+
// Parts:
188+
// - Name: DXIL
189+
// Size: 28
190+
// Program:
191+
// MajorVersion: 6
192+
// MinorVersion: 5
193+
// ShaderKind: 5
194+
// Size: 8
195+
// DXILMajorVersion: 1
196+
// DXILMinorVersion: 5
197+
// DXILSize: 4
198+
// DXIL: [ 0x42, 0x43, 0xC0, 0xDE, ]
199+
// ...
200+
TEST(DXCFile, ParseDXILPart) {
201+
uint8_t Buffer[] = {
202+
0x44, 0x58, 0x42, 0x43, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
203+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
204+
0x48, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
205+
0x44, 0x58, 0x49, 0x4c, 0x1c, 0x00, 0x00, 0x00, 0x65, 0x00, 0x05, 0x00,
206+
0x08, 0x00, 0x00, 0x00, 0x44, 0x58, 0x49, 0x4c, 0x05, 0x01, 0x00, 0x00,
207+
0x10, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x42, 0x43, 0xc0, 0xde};
208+
DXContainer C =
209+
llvm::cantFail(DXContainer::create(getMemoryBuffer<116>(Buffer)));
210+
EXPECT_EQ(C.getHeader().PartCount, 1u);
211+
const std::optional<object::DXContainer::DXILData> &DXIL = C.getDXIL();
212+
EXPECT_TRUE(DXIL.has_value());
213+
dxbc::ProgramHeader Header = DXIL->first;
214+
EXPECT_EQ(Header.getMajorVersion(), 6u);
215+
EXPECT_EQ(Header.getMinorVersion(), 5u);
216+
EXPECT_EQ(Header.ShaderKind, 5u);
217+
EXPECT_EQ(Header.Size, 8u);
218+
}
219+
177220
static Expected<DXContainer>
178221
generateDXContainer(StringRef Yaml, SmallVectorImpl<char> &BinaryData) {
179222
DXContainerYAML::Object Obj;

0 commit comments

Comments
 (0)