Skip to content

Commit 36746f5

Browse files
author
joaosaffran
committed
addressing pr comments
1 parent d6c98ed commit 36746f5

File tree

5 files changed

+37
-75
lines changed

5 files changed

+37
-75
lines changed

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -550,13 +550,10 @@ static_assert(sizeof(ProgramSignatureElement) == 32,
550550

551551
struct RootSignatureValidations {
552552

553-
static bool validateRootFlag(uint32_t Flags) { return (Flags & ~0xfff) != 0; }
553+
static bool isValidRootFlag(uint32_t Flags) { return (Flags & ~0xfff) == 0; }
554554

555-
static Expected<uint32_t> validateVersion(uint32_t Version) {
556-
if (Version < 1 || Version > 2)
557-
return llvm::make_error<BinaryStreamError>(
558-
"Invalid Root Signature Version");
559-
return Version;
555+
static bool isValidVersion(uint32_t Version) {
556+
return (Version == 1 || Version == 2);
560557
}
561558
};
562559

‎llvm/lib/Object/DXContainer.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,8 @@ Error DirectX::RootSignature::parse(StringRef Data) {
258258
support::endian::read<uint32_t, llvm::endianness::little>(Current);
259259
Current += sizeof(uint32_t);
260260

261-
if (dxbc::RootSignatureValidations::validateVersion(VValue))
262-
return make_error<GenericBinaryError>("Invalid Version");
261+
if (!dxbc::RootSignatureValidations::isValidVersion(VValue))
262+
return make_error<GenericBinaryError>("Invalid Root Signature Version");
263263
Version = VValue;
264264

265265
NumParameters =
@@ -282,8 +282,8 @@ Error DirectX::RootSignature::parse(StringRef Data) {
282282
support::endian::read<uint32_t, llvm::endianness::little>(Current);
283283
Current += sizeof(uint32_t);
284284

285-
if (dxbc::RootSignatureValidations::validateRootFlag(FValue))
286-
return make_error<GenericBinaryError>("Invalid flag");
285+
if (!dxbc::RootSignatureValidations::isValidRootFlag(FValue))
286+
return make_error<GenericBinaryError>("Invalid Root Signature flag");
287287
Flags = FValue;
288288

289289
return Error::success();

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -155,18 +155,14 @@ void DXContainerGlobals::addRootSignature(Module &M,
155155
SmallVector<GlobalValue *> &Globals) {
156156

157157
auto &RSA = getAnalysis<RootSignatureAnalysisWrapper>();
158-
std::optional<ModuleRootSignature> MaybeRootSignature = RSA.getResult();
159-
160-
if (!MaybeRootSignature.has_value())
158+
if (!RSA.getResult())
161159
return;
162160

163-
ModuleRootSignature MRS = MaybeRootSignature.value();
164-
165161
SmallString<256> Data;
166162
raw_svector_ostream OS(Data);
167163

168164
RootSignatureHeader RSH;
169-
RSH.Flags = MRS.Flags;
165+
RSH.Flags = RSA.getResult()->Flags;
170166

171167
RSH.write(OS);
172168

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

Lines changed: 23 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -57,24 +57,13 @@ static bool parseRootSignatureElement(LLVMContext *Ctx,
5757
RootSignatureElementKind ElementKind =
5858
StringSwitch<RootSignatureElementKind>(ElementText->getString())
5959
.Case("RootFlags", RootSignatureElementKind::RootFlags)
60-
.Case("RootConstants", RootSignatureElementKind::RootConstants)
61-
.Case("RootCBV", RootSignatureElementKind::RootDescriptor)
62-
.Case("RootSRV", RootSignatureElementKind::RootDescriptor)
63-
.Case("RootUAV", RootSignatureElementKind::RootDescriptor)
64-
.Case("Sampler", RootSignatureElementKind::RootDescriptor)
65-
.Case("DescriptorTable", RootSignatureElementKind::DescriptorTable)
66-
.Case("StaticSampler", RootSignatureElementKind::StaticSampler)
6760
.Default(RootSignatureElementKind::None);
6861

6962
switch (ElementKind) {
7063

7164
case RootSignatureElementKind::RootFlags:
7265
return parseRootFlags(Ctx, MRS, Element);
73-
case RootSignatureElementKind::RootConstants:
74-
case RootSignatureElementKind::RootDescriptor:
75-
case RootSignatureElementKind::DescriptorTable:
76-
case RootSignatureElementKind::StaticSampler:
77-
case RootSignatureElementKind::None:
66+
default:
7867
return reportError(Ctx,
7968
"Invalid Root Element: " + ElementText->getString());
8069
}
@@ -83,7 +72,7 @@ static bool parseRootSignatureElement(LLVMContext *Ctx,
8372
}
8473

8574
static bool parse(LLVMContext *Ctx, ModuleRootSignature *MRS, NamedMDNode *Root,
86-
const Function *EF) {
75+
const Function *EntryFunction) {
8776
bool HasError = false;
8877

8978
/** Root Signature are specified as following in the metadata:
@@ -121,7 +110,7 @@ static bool parse(LLVMContext *Ctx, ModuleRootSignature *MRS, NamedMDNode *Root,
121110
continue;
122111
}
123112

124-
if (F != EF)
113+
if (F != EntryFunction)
125114
continue;
126115

127116
// Get the Root Signature Description from the function signature pair.
@@ -133,8 +122,8 @@ static bool parse(LLVMContext *Ctx, ModuleRootSignature *MRS, NamedMDNode *Root,
133122
}
134123

135124
// Loop through the Root Elements of the root signature.
136-
for (unsigned int Eid = 0; Eid < RS->getNumOperands(); Eid++) {
137-
MDNode *Element = dyn_cast<MDNode>(RS->getOperand(Eid));
125+
for (const auto &Operand : RS->operands()) {
126+
MDNode *Element = dyn_cast<MDNode>(Operand);
138127
if (Element == nullptr)
139128
return reportError(Ctx, "Missing Root Element Metadata Node.");
140129

@@ -145,20 +134,30 @@ static bool parse(LLVMContext *Ctx, ModuleRootSignature *MRS, NamedMDNode *Root,
145134
}
146135

147136
static bool validate(LLVMContext *Ctx, ModuleRootSignature *MRS) {
148-
if (dxbc::RootSignatureValidations::validateRootFlag(MRS->Flags)) {
137+
if (!dxbc::RootSignatureValidations::isValidRootFlag(MRS->Flags)) {
149138
return reportError(Ctx, "Invalid Root Signature flag value");
150139
}
151140
return false;
152141
}
153142

154143
std::optional<ModuleRootSignature>
155-
ModuleRootSignature::analyzeModule(Module &M, const Function *F) {
156-
ModuleRootSignature MRS;
144+
ModuleRootSignature::analyzeModule(Module &M, ModuleMetadataInfo MMI) {
145+
if (MMI.ShaderProfile == Triple::Library)
146+
return std::nullopt;
147+
157148
LLVMContext *Ctx = &M.getContext();
158149

150+
if (MMI.EntryPropertyVec.size() != 1) {
151+
reportError(Ctx, "More than one entry function defined.");
152+
return std::nullopt;
153+
}
154+
155+
ModuleRootSignature MRS;
156+
const Function *EntryFunction = MMI.EntryPropertyVec[0].Entry;
157+
159158
NamedMDNode *RootSignatureNode = M.getNamedMetadata("dx.rootsignatures");
160-
if (RootSignatureNode == nullptr || parse(Ctx, &MRS, RootSignatureNode, F) ||
161-
validate(Ctx, &MRS))
159+
if (RootSignatureNode == nullptr ||
160+
parse(Ctx, &MRS, RootSignatureNode, EntryFunction) || validate(Ctx, &MRS))
162161
return std::nullopt;
163162

164163
return MRS;
@@ -168,39 +167,16 @@ AnalysisKey RootSignatureAnalysis::Key;
168167

169168
std::optional<ModuleRootSignature>
170169
RootSignatureAnalysis::run(Module &M, ModuleAnalysisManager &AM) {
171-
auto MMI = AM.getResult<DXILMetadataAnalysis>(M);
172-
173-
if (MMI.ShaderProfile == Triple::Library)
174-
return std::nullopt;
175-
176-
LLVMContext *Ctx = &M.getContext();
177-
178-
if (MMI.EntryPropertyVec.size() != 1) {
179-
reportError(Ctx, "More than one entry function defined.");
180-
return std::nullopt;
181-
}
182-
183-
const Function *EntryFunction = MMI.EntryPropertyVec[0].Entry;
184-
return ModuleRootSignature::analyzeModule(M, EntryFunction);
170+
ModuleMetadataInfo MMI = AM.getResult<DXILMetadataAnalysis>(M);
171+
return ModuleRootSignature::analyzeModule(M, MMI);
185172
}
186173

187174
//===----------------------------------------------------------------------===//
188175
bool RootSignatureAnalysisWrapper::runOnModule(Module &M) {
189176

190177
dxil::ModuleMetadataInfo &MMI =
191178
getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata();
192-
193-
if (MMI.ShaderProfile == Triple::Library)
194-
return false;
195-
196-
LLVMContext *Ctx = &M.getContext();
197-
if (MMI.EntryPropertyVec.size() != 1) {
198-
reportError(Ctx, "More than one entry function defined.");
199-
return false;
200-
}
201-
202-
const Function *EntryFunction = MMI.EntryPropertyVec[0].Entry;
203-
MRS = ModuleRootSignature::analyzeModule(M, EntryFunction);
179+
MRS = ModuleRootSignature::analyzeModule(M, MMI);
204180
return false;
205181
}
206182

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

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
//===- DXILRootSignature.h - DXIL Root Signature helper objects
2-
//---------------===//
1+
//===- DXILRootSignature.h - DXIL Root Signature helper objects -----------===//
32
//
43
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
54
// See https://llvm.org/LICENSE.txt for license information.
@@ -12,6 +11,7 @@
1211
///
1312
//===----------------------------------------------------------------------===//
1413

14+
#include "llvm/Analysis/DXILMetadataAnalysis.h"
1515
#include "llvm/IR/DiagnosticInfo.h"
1616
#include "llvm/IR/Metadata.h"
1717
#include "llvm/IR/Module.h"
@@ -22,19 +22,12 @@
2222
namespace llvm {
2323
namespace dxil {
2424

25-
enum class RootSignatureElementKind {
26-
None = 0,
27-
RootFlags = 1,
28-
RootConstants = 2,
29-
RootDescriptor = 3,
30-
DescriptorTable = 4,
31-
StaticSampler = 5
32-
};
25+
enum class RootSignatureElementKind { None = 0, RootFlags = 1 };
3326

3427
struct ModuleRootSignature {
3528
uint32_t Flags = 0;
36-
static std::optional<ModuleRootSignature> analyzeModule(Module &M,
37-
const Function *F);
29+
static std::optional<ModuleRootSignature>
30+
analyzeModule(Module &M, ModuleMetadataInfo MMI);
3831
};
3932

4033
class RootSignatureAnalysis : public AnalysisInfoMixin<RootSignatureAnalysis> {

0 commit comments

Comments
 (0)