Skip to content

Commit 01b49a7

Browse files
author
joaosaffran
committed
addressing pr comments
1 parent b175b65 commit 01b49a7

File tree

5 files changed

+35
-70
lines changed

5 files changed

+35
-70
lines changed

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

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

549549
struct RootSignatureValidations {
550550

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

553-
static bool validateVersion(uint32_t Version) {
554-
return !(Version == 1 || Version == 2);
553+
static bool isValidVersion(uint32_t Version) {
554+
return (Version == 1 || Version == 2);
555555
}
556556
};
557557

‎llvm/lib/Object/DXContainer.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ Error DirectX::RootSignature::parse(StringRef Data) {
254254
support::endian::read<uint32_t, llvm::endianness::little>(Current);
255255
Current += sizeof(uint32_t);
256256

257-
if (dxbc::RootSignatureValidations::validateVersion(VValue))
257+
if (!dxbc::RootSignatureValidations::isValidVersion(VValue))
258258
return make_error<GenericBinaryError>("Invalid Root Signature Version");
259259
Version = VValue;
260260

@@ -278,7 +278,7 @@ Error DirectX::RootSignature::parse(StringRef Data) {
278278
support::endian::read<uint32_t, llvm::endianness::little>(Current);
279279
Current += sizeof(uint32_t);
280280

281-
if (dxbc::RootSignatureValidations::validateRootFlag(FValue))
281+
if (!dxbc::RootSignatureValidations::isValidRootFlag(FValue))
282282
return make_error<GenericBinaryError>("Invalid Root Signature flag");
283283
Flags = FValue;
284284

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

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

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

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

167163
RootSignatureHeader RSH;
168-
RSH.Flags = MRS.Flags;
164+
RSH.Flags = RSA.getResult()->Flags;
169165

170166
RSH.write(OS);
171167

‎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)