Skip to content

Commit 1026a8e

Browse files
author
joaosaffran
committed
multiple parameters support and more testing
1 parent b232967 commit 1026a8e

File tree

7 files changed

+191
-31
lines changed

7 files changed

+191
-31
lines changed

‎llvm/include/llvm/BinaryFormat/DXContainer.h

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@
1414
#define LLVM_BINARYFORMAT_DXCONTAINER_H
1515

1616
#include "llvm/ADT/StringRef.h"
17-
#include "llvm/Support/BinaryStreamError.h"
18-
#include "llvm/Support/Error.h"
17+
#include "llvm/Support/ErrorHandling.h"
1918
#include "llvm/Support/SwapByteOrder.h"
2019
#include "llvm/TargetParser/Triple.h"
2120

21+
#include <cstdint>
2222
#include <stdint.h>
2323

2424
namespace llvm {
@@ -563,9 +563,11 @@ static_assert(sizeof(ProgramSignatureElement) == 32,
563563
"ProgramSignatureElement is misaligned");
564564

565565
struct RootConstants {
566-
uint32_t ShaderRegister;
567-
uint32_t RegisterSpace;
568-
uint32_t Num32BitValues;
566+
uint32_t ShaderRegister = 0;
567+
uint32_t RegisterSpace = 0;
568+
uint32_t Num32BitValues = 0;
569+
570+
RootConstants() = default;
569571

570572
void swapBytes() {
571573
sys::swapByteOrder(ShaderRegister);
@@ -581,23 +583,35 @@ struct RootParameter {
581583
};
582584
dxbc::ShaderVisibilityFlag ShaderVisibility;
583585

586+
RootParameter() {
587+
Constants = RootConstants();
588+
ParameterType = dxbc::RootParameterType::Empty;
589+
ShaderVisibility = dxbc::ShaderVisibilityFlag::Empty;
590+
}
591+
584592
void swapBytes() {
593+
585594
sys::swapByteOrder(ParameterType);
586595
sys::swapByteOrder(ShaderVisibility);
587596
switch (ParameterType) {
588597

589598
case RootParameterType::Constants32Bit:
590599
Constants.swapBytes();
591600
break;
601+
case RootParameterType::Empty:
602+
llvm_unreachable("invalid value for ParameterType");
603+
break;
592604
}
593605
}
594606
};
595607

596608
struct RootSignatureHeader {
597609
uint32_t Version = 2;
598610
uint32_t Flags = 0;
611+
599612
void swapBytes() {
600-
// nothing to swap
613+
sys::swapByteOrder(Version);
614+
sys::swapByteOrder(Flags);
601615
}
602616
};
603617

@@ -608,6 +622,16 @@ struct RootSignatureValidations {
608622
static bool isValidVersion(uint32_t Version) {
609623
return (Version == 1 || Version == 2);
610624
}
625+
626+
static bool isValidParameterType(dxbc::RootParameterType Flag) {
627+
// RootParameterType::Empty is the higest value in the enum.
628+
return Flag < dxbc::RootParameterType::Empty;
629+
}
630+
631+
static bool isValidShaderVisibility(dxbc::ShaderVisibilityFlag Flag) {
632+
// ShaderVisibilityFlag::Empty is the higest value in the enum.
633+
return Flag < dxbc::ShaderVisibilityFlag::Empty;
634+
}
611635
};
612636

613637
} // namespace dxbc

‎llvm/include/llvm/BinaryFormat/DXContainerConstants.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ ROOT_ELEMENT_FLAG(11, SamplerHeapDirectlyIndexed)
7575
#ifdef ROOT_PARAMETER
7676

7777
ROOT_PARAMETER(1, Constants32Bit)
78+
ROOT_PARAMETER(5, Empty)
79+
7880
#undef ROOT_PARAMETER
7981
#endif // ROOT_PARAMETER
8082

@@ -87,6 +89,7 @@ SHADER_VISIBILITY(4, Geometry)
8789
SHADER_VISIBILITY(5, Pixel)
8890
SHADER_VISIBILITY(6, Amplification)
8991
SHADER_VISIBILITY(7, Mesh)
92+
SHADER_VISIBILITY(8, Empty)
9093
#undef SHADER_VISIBILITY
9194
#endif // SHADER_VISIBILITY
9295

‎llvm/lib/MC/DXContainerRootSignature.cpp

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,47 +10,41 @@
1010
#include "llvm/ADT/bit.h"
1111
#include "llvm/BinaryFormat/DXContainer.h"
1212
#include "llvm/Support/EndianStream.h"
13+
#include "llvm/Support/ErrorHandling.h"
1314
#include <cstdint>
1415
#include <sys/types.h>
1516

1617
using namespace llvm;
1718
using namespace llvm::mcdxbc;
1819

19-
template <typename T> static uint32_t getSizeOf() {
20-
return static_cast<uint32_t>(sizeof(T));
21-
}
22-
2320
void RootSignatureDesc::write(raw_ostream &OS) const {
24-
uint32_t Offset = 16;
25-
const uint32_t ParametersOffset =
26-
getSizeOf<dxbc::RootSignatureHeader>() + Offset;
21+
const uint32_t HeaderSize = 24;
2722
const uint32_t ParameterByteSize = Parameters.size_in_bytes();
2823

2924
// Writing header information
3025
support::endian::write(OS, Header.Version, llvm::endianness::little);
31-
Offset += getSizeOf<uint32_t>();
32-
3326
support::endian::write(OS, (uint32_t)Parameters.size(),
3427
llvm::endianness::little);
35-
Offset += getSizeOf<uint32_t>();
36-
37-
support::endian::write(OS, ParametersOffset, llvm::endianness::little);
38-
Offset += getSizeOf<uint32_t>();
39-
28+
support::endian::write(OS, HeaderSize, llvm::endianness::little);
4029
support::endian::write(OS, ((uint32_t)0), llvm::endianness::little);
41-
Offset += getSizeOf<uint32_t>();
42-
43-
support::endian::write(OS, ParameterByteSize + ParametersOffset,
30+
// TODO: this value means nothing right now...
31+
support::endian::write(OS, ParameterByteSize + HeaderSize,
4432
llvm::endianness::little);
45-
Offset += getSizeOf<uint32_t>();
4633

4734
support::endian::write(OS, Header.Flags, llvm::endianness::little);
4835

36+
uint32_t ParamsOffset =
37+
HeaderSize + (3 * sizeof(uint32_t) * Parameters.size());
4938
for (const dxbc::RootParameter &P : Parameters) {
5039
support::endian::write(OS, P.ParameterType, llvm::endianness::little);
5140
support::endian::write(OS, P.ShaderVisibility, llvm::endianness::little);
52-
support::endian::write(OS, Offset, llvm::endianness::little);
41+
support::endian::write(OS, ParamsOffset, llvm::endianness::little);
42+
43+
// Size of root parameter, removing the ParameterType and ShaderVisibility.
44+
ParamsOffset += sizeof(dxbc::RootParameter) - 2 * sizeof(uint32_t);
45+
}
5346

47+
for (const dxbc::RootParameter &P : Parameters) {
5448
switch (P.ParameterType) {
5549
case dxbc::RootParameterType::Constants32Bit: {
5650
support::endian::write(OS, P.Constants.ShaderRegister,
@@ -59,9 +53,9 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
5953
llvm::endianness::little);
6054
support::endian::write(OS, P.Constants.Num32BitValues,
6155
llvm::endianness::little);
62-
Offset += getSizeOf<dxbc::RootConstants>() + 3 * getSizeOf<uint32_t>();
63-
6456
} break;
57+
case dxbc::RootParameterType::Empty:
58+
llvm_unreachable("Invalid RootParameterType");
6559
}
6660
}
6761
}

‎llvm/lib/Object/DXContainer.cpp

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "llvm/Object/Error.h"
1212
#include "llvm/Support/Alignment.h"
1313
#include "llvm/Support/Endian.h"
14+
#include "llvm/Support/ErrorHandling.h"
1415
#include "llvm/Support/FormatVariadic.h"
1516
#include <cstddef>
1617
#include <cstdint>
@@ -295,14 +296,27 @@ Error DirectX::RootSignature::parse(StringRef Data) {
295296
for (uint32_t It = 0; It < NumParameters; It++) {
296297
dxbc::RootParameter NewParam;
297298

299+
dxbc::RootParameterType PTValue =
300+
support::endian::read<dxbc::RootParameterType,
301+
llvm::endianness::little>(Current);
302+
if (!dxbc::RootSignatureValidations::isValidParameterType(PTValue))
303+
return validationFailed("unsupported parameter type value read: " +
304+
llvm::Twine((uint32_t)PTValue));
305+
298306
NewParam.ParameterType =
299307
support::endian::read<dxbc::RootParameterType,
300308
llvm::endianness::little>(Current);
301309
Current += sizeof(dxbc::RootParameterType);
302310

303-
NewParam.ShaderVisibility =
311+
dxbc::ShaderVisibilityFlag SVValue =
304312
support::endian::read<dxbc::ShaderVisibilityFlag,
305313
llvm::endianness::little>(Current);
314+
315+
if (!dxbc::RootSignatureValidations::isValidShaderVisibility(SVValue))
316+
return validationFailed("unsupported shader visility flag value read: " +
317+
llvm::Twine((uint32_t)SVValue));
318+
319+
NewParam.ShaderVisibility = SVValue;
306320
Current += sizeof(dxbc::ShaderVisibilityFlag);
307321

308322
uint32_t Offset =
@@ -312,11 +326,12 @@ Error DirectX::RootSignature::parse(StringRef Data) {
312326
switch (NewParam.ParameterType) {
313327

314328
case dxbc::RootParameterType::Constants32Bit: {
315-
if (Error Err = readStruct(Data, Current, NewParam.Constants))
329+
if (Error Err = readStruct(Data, Begin + Offset, NewParam.Constants))
316330
return Err;
317-
if (sys::IsBigEndianHost)
318-
NewParam.Constants.swapBytes();
319331
} break;
332+
case dxbc::RootParameterType::Empty:
333+
llvm_unreachable("Invalid value for RootParameterType");
334+
break;
320335
}
321336

322337
Parameters.push_back(NewParam);

‎llvm/lib/ObjectYAML/DXContainerYAML.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "llvm/ObjectYAML/DXContainerYAML.h"
1515
#include "llvm/ADT/ScopeExit.h"
1616
#include "llvm/BinaryFormat/DXContainer.h"
17+
#include "llvm/Support/ErrorHandling.h"
1718
#include "llvm/Support/ScopedPrinter.h"
1819

1920
namespace llvm {
@@ -235,6 +236,9 @@ void MappingTraits<dxbc::RootParameter>::mapping(IO &IO,
235236
case dxbc::RootParameterType::Constants32Bit:
236237
IO.mapRequired("Constants", P.Constants);
237238

239+
break;
240+
case dxbc::RootParameterType::Empty:
241+
llvm_unreachable("Invalid value for ParameterType");
238242
break;
239243
}
240244
}

‎llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# RUN: yaml2obj %s | obj2yaml | FileCheck %s
2+
3+
--- !dxcontainer
4+
Header:
5+
Hash: [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
6+
0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
7+
Version:
8+
Major: 1
9+
Minor: 0
10+
PartCount: 1
11+
PartOffsets: [ 60 ]
12+
Parts:
13+
- Name: RTS0
14+
Size: 80
15+
RootSignature:
16+
Version: 2
17+
NumStaticSamplers: 0
18+
StaticSamplersOffset: 64
19+
Parameters:
20+
- ParameterType: Constants32Bit
21+
ShaderVisibility: Hull
22+
Constants:
23+
Num32BitValues: 16
24+
ShaderRegister: 15
25+
RegisterSpace: 14
26+
- ParameterType: Constants32Bit
27+
ShaderVisibility: Geometry
28+
Constants:
29+
Num32BitValues: 21
30+
ShaderRegister: 22
31+
RegisterSpace: 23
32+
AllowInputAssemblerInputLayout: true
33+
DenyGeometryShaderRootAccess: true
34+
35+
# CHECK: - Name: RTS0
36+
# CHECK-NEXT: Size: 80
37+
# CHECK-NEXT: RootSignature:
38+
# CHECK-NEXT: Version: 2
39+
# CHECK-NEXT: NumStaticSamplers: 0
40+
# CHECK-NEXT: StaticSamplersOffset: 64
41+
# CHECK-NEXT: Parameters:
42+
# CHECK-NEXT: - ParameterType: Constants32Bit
43+
# CHECK-NEXT: ShaderVisibility: Hull
44+
# CHECK-NEXT: Constants:
45+
# CHECK-NEXT: Num32BitValues: 16
46+
# CHECK-NEXT: RegisterSpace: 14
47+
# CHECK-NEXT: ShaderRegister: 15
48+
# CHECK-NEXT: - ParameterType: Constants32Bit
49+
# CHECK-NEXT: ShaderVisibility: Geometry
50+
# CHECK-NEXT: Constants:
51+
# CHECK-NEXT: Num32BitValues: 21
52+
# CHECK-NEXT: RegisterSpace: 23
53+
# CHECK-NEXT: ShaderRegister: 22
54+
# CHECK-NEXT: AllowInputAssemblerInputLayout: true
55+
# CHECK-NEXT: DenyGeometryShaderRootAccess: true

‎llvm/unittests/Object/DXContainerTest.cpp

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "llvm/Support/MemoryBufferRef.h"
1515
#include "llvm/Testing/Support/Error.h"
1616
#include "gtest/gtest.h"
17+
#include <cstdint>
1718

1819
using namespace llvm;
1920
using namespace llvm::object;
@@ -917,5 +918,69 @@ TEST(RootSignature, ParseRootConstant) {
917918
ASSERT_EQ(RS->getNumStaticSamplers(), 0u);
918919
ASSERT_EQ(RS->getStaticSamplersOffset(), 44u);
919920
ASSERT_EQ(RS->getFlags(), 17u);
921+
922+
const auto RootParam = RS->getParameters()[0];
923+
ASSERT_EQ((uint32_t)RootParam.ParameterType, 1u);
924+
ASSERT_EQ((uint32_t)RootParam.ShaderVisibility, 2u);
925+
ASSERT_EQ(RootParam.Constants.ShaderRegister, 15u);
926+
ASSERT_EQ(RootParam.Constants.RegisterSpace, 14u);
927+
ASSERT_EQ(RootParam.Constants.Num32BitValues, 16u);
928+
}
929+
{
930+
// ParameterType has been set to an invalid value
931+
uint8_t Buffer[] = {
932+
0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f,
933+
0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00,
934+
0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
935+
0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
936+
0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
937+
0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0xFF, 0x00, 0x00, 0x00,
938+
0x02, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00,
939+
0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
940+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
941+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
942+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
943+
0x00};
944+
EXPECT_THAT_EXPECTED(
945+
DXContainer::create(getMemoryBuffer<133>(Buffer)),
946+
FailedWithMessage("unsupported parameter type value read: 255"));
947+
}
948+
{
949+
// ShaderVisibility has been set to an invalid value
950+
uint8_t Buffer[] = {
951+
0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f,
952+
0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00,
953+
0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
954+
0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
955+
0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
956+
0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
957+
0xFF, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00,
958+
0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
959+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
960+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
961+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
962+
0x00};
963+
EXPECT_THAT_EXPECTED(
964+
DXContainer::create(getMemoryBuffer<133>(Buffer)),
965+
FailedWithMessage("unsupported shader visility flag value read: 255"));
966+
}
967+
{
968+
// Offset has been set to an invalid value
969+
uint8_t Buffer[] = {
970+
0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f,
971+
0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00,
972+
0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
973+
0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
974+
0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
975+
0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
976+
0x02, 0x00, 0x00, 0x00, 0x4e, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00,
977+
0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
978+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
979+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
980+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
981+
0x00};
982+
EXPECT_THAT_EXPECTED(
983+
DXContainer::create(getMemoryBuffer<133>(Buffer)),
984+
FailedWithMessage("Reading structure out of file bounds"));
920985
}
921986
}

0 commit comments

Comments
 (0)