Skip to content

Commit 39d4b08

Browse files
author
joaosaffran
committed
addressing pr comments
1 parent 102ff4b commit 39d4b08

File tree

9 files changed

+88
-48
lines changed

9 files changed

+88
-48
lines changed

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

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

1616
#include "llvm/ADT/StringRef.h"
17+
#include "llvm/Support/Error.h"
1718
#include "llvm/Support/SwapByteOrder.h"
1819
#include "llvm/TargetParser/Triple.h"
1920

@@ -158,19 +159,43 @@ enum class RootElementFlag : uint32_t {
158159
};
159160

160161
#define ROOT_PARAMETER(Val, Enum) Enum = Val,
161-
enum class RootParameterType : uint32_t {
162+
enum RootParameterType : uint32_t {
162163
#include "DXContainerConstants.def"
163164
};
164165

165166
ArrayRef<EnumEntry<RootParameterType>> getRootParameterTypes();
166167

168+
#define ROOT_PARAMETER(Val, Enum) \
169+
case Val: \
170+
return dxbc::RootParameterType::Enum;
171+
inline llvm::Expected<dxbc::RootParameterType>
172+
safeParseParameterType(uint32_t V) {
173+
switch (V) {
174+
#include "DXContainerConstants.def"
175+
}
176+
return createStringError(std::errc::invalid_argument,
177+
"Invalid value for parameter type");
178+
}
179+
167180
#define SHADER_VISIBILITY(Val, Enum) Enum = Val,
168-
enum class ShaderVisibility : uint32_t {
181+
enum ShaderVisibility : uint32_t {
169182
#include "DXContainerConstants.def"
170183
};
171184

172185
ArrayRef<EnumEntry<ShaderVisibility>> getShaderVisibility();
173186

187+
#define SHADER_VISIBILITY(Val, Enum) \
188+
case Val: \
189+
return dxbc::ShaderVisibility::Enum;
190+
inline llvm::Expected<dxbc::ShaderVisibility>
191+
safeParseShaderVisibility(uint32_t V) {
192+
switch (V) {
193+
#include "DXContainerConstants.def"
194+
}
195+
return createStringError(std::errc::invalid_argument,
196+
"Invalid value for parameter type");
197+
}
198+
174199
PartType parsePartType(StringRef S);
175200

176201
struct VertexPSVInfo {
@@ -575,8 +600,8 @@ struct RootConstants {
575600
};
576601

577602
struct RootParameterHeader {
578-
RootParameterType ParameterType;
579-
ShaderVisibility ShaderVisibility;
603+
uint32_t ParameterType;
604+
uint32_t ShaderVisibility;
580605
uint32_t ParameterOffset;
581606

582607
void swapBytes() {

‎llvm/include/llvm/MC/DXContainerRootSignature.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,9 @@ struct RootParameter {
2323
};
2424
struct RootSignatureDesc {
2525

26-
dxbc::RootSignatureHeader Header;
26+
uint32_t Version = 2U;
27+
uint32_t Flags = 0U;
2728
SmallVector<mcdxbc::RootParameter> Parameters;
28-
RootSignatureDesc()
29-
: Header(dxbc::RootSignatureHeader{
30-
2, 0, sizeof(dxbc::RootSignatureHeader), 0, 0, 0}) {}
3129

3230
void write(raw_ostream &OS) const;
3331

‎llvm/include/llvm/Object/DXContainer.h‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -117,16 +117,6 @@ template <typename T> struct ViewArray {
117117
};
118118

119119
namespace DirectX {
120-
121-
struct RootParameter {
122-
dxbc::RootParameterHeader Header;
123-
union {
124-
dxbc::RootConstants Constants;
125-
};
126-
127-
RootParameter() = default;
128-
};
129-
130120
struct RootParameterView {
131121
const dxbc::RootParameterHeader &Header;
132122
StringRef ParamData;
@@ -183,7 +173,7 @@ class RootSignature {
183173
uint32_t getRootParametersOffset() const { return RootParametersOffset; }
184174
uint32_t getNumStaticSamplers() const { return NumStaticSamplers; }
185175
uint32_t getStaticSamplersOffset() const { return StaticSamplersOffset; }
186-
llvm::iterator_range<param_header_iterator> param_header() const {
176+
llvm::iterator_range<param_header_iterator> param_headers() const {
187177
return llvm::make_range(ParametersHeaders.begin(), ParametersHeaders.end());
188178
}
189179
uint32_t getFlags() const { return Flags; }

‎llvm/include/llvm/ObjectYAML/DXContainerYAML.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ struct RootSignatureYamlDesc {
103103

104104
uint32_t getEncodedFlags();
105105

106+
iterator_range<RootParameterYamlDesc *> params() {
107+
return make_range(Parameters.begin(), Parameters.end());
108+
}
109+
106110
#include "llvm/BinaryFormat/DXContainerConstants.def"
107111
};
108112

‎llvm/lib/MC/DXContainerRootSignature.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,13 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
5151
const uint32_t StaticSamplerOffset = 0u;
5252
const uint32_t NumStaticSamplers = 0u;
5353

54-
support::endian::write(BOS, Header.Version, llvm::endianness::little);
54+
support::endian::write(BOS, Version, llvm::endianness::little);
5555
support::endian::write(BOS, NumParameters, llvm::endianness::little);
5656
support::endian::write(BOS, (uint32_t)sizeof(dxbc::RootSignatureHeader),
5757
llvm::endianness::little);
5858
support::endian::write(BOS, StaticSamplerOffset, llvm::endianness::little);
5959
support::endian::write(BOS, NumStaticSamplers, llvm::endianness::little);
60-
support::endian::write(BOS, Header.Flags, llvm::endianness::little);
60+
support::endian::write(BOS, Flags, llvm::endianness::little);
6161

6262
SmallVector<uint32_t> ParamsOffsets;
6363
for (const auto &P : Parameters) {

‎llvm/lib/ObjectYAML/DXContainerEmitter.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,8 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
267267
continue;
268268

269269
mcdxbc::RootSignatureDesc RS;
270-
RS.Header.Flags = P.RootSignature->getEncodedFlags();
271-
RS.Header.Version = P.RootSignature->Version;
270+
RS.Flags = P.RootSignature->getEncodedFlags();
271+
RS.Version = P.RootSignature->Version;
272272
for (const auto &Param : P.RootSignature->Parameters) {
273273
mcdxbc::RootParameter NewParam;
274274
NewParam.Header = dxbc::RootParameterHeader{

‎llvm/lib/ObjectYAML/DXContainerYAML.cpp‎

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,27 +35,48 @@ DXContainerYAML::RootSignatureYamlDesc::RootSignatureYamlDesc(
3535
NumStaticSamplers(Data.getNumStaticSamplers()),
3636
StaticSamplersOffset(Data.getStaticSamplersOffset()) {
3737
uint32_t Flags = Data.getFlags();
38-
for (const auto &PH : Data.param_header()) {
38+
for (const auto &PH : Data.param_headers()) {
3939

4040
RootParameterYamlDesc NewP;
4141
NewP.Offset = PH.ParameterOffset;
42-
NewP.Type = PH.ParameterType;
43-
NewP.Visibility = PH.ShaderVisibility;
4442

45-
llvm::Expected<object::DirectX::RootParameterView> ParamView =
43+
llvm::Expected<dxbc::RootParameterType> TypeOrErr =
44+
dxbc::safeParseParameterType(PH.ParameterType);
45+
if (Error E = TypeOrErr.takeError()) {
46+
llvm::errs() << "Error: " << E << "\n";
47+
continue;
48+
}
49+
50+
NewP.Type = TypeOrErr.get();
51+
52+
llvm::Expected<dxbc::ShaderVisibility> VisibilityOrErr =
53+
dxbc::safeParseShaderVisibility(PH.ShaderVisibility);
54+
if (Error E = VisibilityOrErr.takeError()) {
55+
llvm::errs() << "Error: " << E << "\n";
56+
continue;
57+
}
58+
NewP.Visibility = VisibilityOrErr.get();
59+
60+
llvm::Expected<object::DirectX::RootParameterView> ParamViewOrErr =
4661
Data.getParameter(PH);
47-
if (!ParamView)
48-
llvm::errs() << "Error: " << ParamView.takeError() << "\n";
49-
auto PV = *ParamView;
50-
51-
if (auto *RCV = dyn_cast<object::DirectX::RootConstantView>(&PV)) {
52-
auto Constants = RCV->read();
53-
if (!Constants)
54-
llvm::errs() << "Error: " << Constants.takeError() << "\n";
55-
56-
NewP.Constants.Num32BitValues = Constants->Num32BitValues;
57-
NewP.Constants.ShaderRegister = Constants->ShaderRegister;
58-
NewP.Constants.RegisterSpace = Constants->RegisterSpace;
62+
if (Error E = ParamViewOrErr.takeError()) {
63+
llvm::errs() << "Error: " << E << "\n";
64+
continue;
65+
}
66+
object::DirectX::RootParameterView ParamView = ParamViewOrErr.get();
67+
68+
if (auto *RCV = dyn_cast<object::DirectX::RootConstantView>(&ParamView)) {
69+
llvm::Expected<dxbc::RootConstants> ConstantsOrErr = RCV->read();
70+
if (Error E = ConstantsOrErr.takeError()) {
71+
llvm::errs() << "Error: " << E << "\n";
72+
continue;
73+
}
74+
75+
auto Constants = *ConstantsOrErr;
76+
77+
NewP.Constants.Num32BitValues = Constants.Num32BitValues;
78+
NewP.Constants.ShaderRegister = Constants.ShaderRegister;
79+
NewP.Constants.RegisterSpace = Constants.RegisterSpace;
5980
}
6081
Parameters.push_back(NewP);
6182
}

‎llvm/lib/Target/DirectX/DXILRootSignature.cpp‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ static bool parseRootFlags(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD,
4747
return reportError(Ctx, "Invalid format for RootFlag Element");
4848

4949
auto *Flag = mdconst::extract<ConstantInt>(RootFlagNode->getOperand(1));
50-
RSD.Header.Flags = Flag->getZExtValue();
50+
RSD.Flags = Flag->getZExtValue();
5151

5252
return false;
5353
}
@@ -95,7 +95,7 @@ static bool parse(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD,
9595
static bool verifyRootFlag(uint32_t Flags) { return (Flags & ~0xfff) == 0; }
9696

9797
static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) {
98-
if (!verifyRootFlag(RSD.Header.Flags)) {
98+
if (!verifyRootFlag(RSD.Flags)) {
9999
return reportError(Ctx, "Invalid Root Signature flag value");
100100
}
101101
return false;
@@ -191,6 +191,8 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M,
191191

192192
SmallDenseMap<const Function *, mcdxbc::RootSignatureDesc> &RSDMap =
193193
AM.getResult<RootSignatureAnalysis>(M);
194+
195+
const size_t RSHSize = sizeof(dxbc::RootSignatureHeader);
194196
OS << "Root Signature Definitions"
195197
<< "\n";
196198
uint8_t Space = 0;
@@ -203,14 +205,14 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M,
203205

204206
// start root signature header
205207
Space++;
206-
OS << indent(Space) << "Flags: " << format_hex(RS.Header.Flags, 8) << ":\n";
207-
OS << indent(Space) << "Version: " << RS.Header.Version << ":\n";
208+
OS << indent(Space) << "Flags: " << format_hex(RS.Flags, 8) << ":\n";
209+
OS << indent(Space) << "Version: " << RS.Version << ":\n";
208210
OS << indent(Space) << "NumParameters: " << RS.Parameters.size() << ":\n";
209-
OS << indent(Space) << "RootParametersOffset: " << sizeof(RS.Header)
210-
<< ":\n";
211+
OS << indent(Space) << "RootParametersOffset: " << RSHSize << ":\n";
211212
OS << indent(Space) << "NumStaticSamplers: " << 0 << ":\n";
212-
OS << indent(Space) << "StaticSamplersOffset: "
213-
<< sizeof(RS.Header) + RS.Parameters.size_in_bytes() << ":\n";
213+
OS << indent(Space)
214+
<< "StaticSamplersOffset: " << RSHSize + RS.Parameters.size_in_bytes()
215+
<< ":\n";
214216
Space--;
215217
// end root signature header
216218
}

‎llvm/unittests/Object/DXContainerTest.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -890,7 +890,7 @@ TEST(RootSignature, ParseRootConstant) {
890890
ASSERT_EQ(RS.getStaticSamplersOffset(), 44u);
891891
ASSERT_EQ(RS.getFlags(), 17u);
892892

893-
auto RootParam = *RS.param_header().begin();
893+
auto RootParam = *RS.param_headers().begin();
894894
ASSERT_EQ((unsigned)RootParam.ParameterType, 1u);
895895
ASSERT_EQ((unsigned)RootParam.ShaderVisibility, 2u);
896896
auto ParamView = RS.getParameter(RootParam);

0 commit comments

Comments
 (0)