Skip to content

Commit f16432f

Browse files
author
joaosaffran
committed
addressing comments
1 parent d59738d commit f16432f

File tree

6 files changed

+132
-32
lines changed

6 files changed

+132
-32
lines changed

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

Lines changed: 57 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
//===- DXILRootSignature.cpp - DXIL Root Signature helper objects
2-
//---------------===//
1+
//===- DXILRootSignature.cpp - 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.
@@ -14,23 +13,31 @@
1413
#include "DXILRootSignature.h"
1514
#include "DirectX.h"
1615
#include "llvm/ADT/StringSwitch.h"
16+
#include "llvm/ADT/Twine.h"
1717
#include "llvm/IR/Constants.h"
18-
#include "llvm/IR/Metadata.h"
1918
#include "llvm/IR/Module.h"
19+
#include <cstdint>
2020

2121
using namespace llvm;
2222
using namespace llvm::dxil;
2323

24+
static bool reportError(Twine Message) {
25+
report_fatal_error(Message, false);
26+
return true;
27+
}
28+
2429
static bool parseRootFlags(ModuleRootSignature *MRS, MDNode *RootFlagNode) {
2530

26-
assert(RootFlagNode->getNumOperands() == 2 &&
27-
"Invalid format for RootFlag Element");
31+
if (RootFlagNode->getNumOperands() != 2)
32+
return reportError("Invalid format for RootFlag Element");
33+
2834
auto *Flag = mdconst::extract<ConstantInt>(RootFlagNode->getOperand(1));
29-
auto Value = Flag->getZExtValue();
35+
uint32_t Value = Flag->getZExtValue();
3036

3137
// Root Element validation, as specified:
3238
// https://github.com/llvm/wg-hlsl/blob/main/proposals/0002-root-signature-in-clang.md#validations-during-dxil-generation
33-
assert((Value & ~0x80000fff) == 0 && "Invalid flag for RootFlag Element");
39+
if ((Value & ~0x80000fff) != 0)
40+
return reportError("Invalid flag value for RootFlag");
3441

3542
MRS->Flags = Value;
3643
return false;
@@ -39,8 +46,8 @@ static bool parseRootFlags(ModuleRootSignature *MRS, MDNode *RootFlagNode) {
3946
static bool parseRootSignatureElement(ModuleRootSignature *MRS,
4047
MDNode *Element) {
4148
MDString *ElementText = cast<MDString>(Element->getOperand(0));
42-
assert(ElementText != nullptr &&
43-
"First preoperty of element is not a string");
49+
if (ElementText == nullptr)
50+
return reportError("Invalid format for Root Element");
4451

4552
RootSignatureElementKind ElementKind =
4653
StringSwitch<RootSignatureElementKind>(ElementText->getString())
@@ -66,7 +73,7 @@ static bool parseRootSignatureElement(ModuleRootSignature *MRS,
6673
case RootSignatureElementKind::DescriptorTable:
6774
case RootSignatureElementKind::StaticSampler:
6875
case RootSignatureElementKind::None:
69-
llvm_unreachable("Not Implemented yet");
76+
return reportError("Invalid Root Element: " + ElementText->getString());
7077
break;
7178
}
7279

@@ -77,49 +84,67 @@ bool ModuleRootSignature::parse(int32_t Version, NamedMDNode *Root) {
7784
this->Version = Version;
7885
bool HasError = false;
7986

87+
/** Root Signature are specified as following in the metadata:
88+
89+
!dx.rootsignatures = !{!2} ; list of function/root signature pairs
90+
!2 = !{ ptr @main, !3 } ; function, root signature
91+
!3 = !{ !4, !5, !6, !7 } ; list of root signature elements
92+
93+
So for each MDNode inside dx.rootsignatures NamedMDNode
94+
(the Root parameter of this function), the parsing process needs
95+
to loop through each of it's operand and process the pairs function
96+
signature pair.
97+
*/
98+
8099
for (unsigned int Sid = 0; Sid < Root->getNumOperands(); Sid++) {
81-
// This should be an if, for error handling
82-
MDNode *Node = cast<MDNode>(Root->getOperand(Sid));
100+
MDNode *Node = dyn_cast<MDNode>(Root->getOperand(Sid));
101+
102+
if (Node == nullptr || Node->getNumOperands() != 2)
103+
return reportError("Invalid format for Root Signature Definition. Pairs "
104+
"of function, root signature expected.");
105+
106+
// Get the Root Signature Description from the function signature pair.
107+
MDNode *RS = dyn_cast<MDNode>(Node->getOperand(1).get());
83108

84-
// Not sure what use this for...
85-
// Metadata *Func = Node->getOperand(0).get();
109+
if (RS == nullptr)
110+
return reportError("Missing Root Signature Metadata node.");
86111

87-
MDNode *Elements = cast<MDNode>(Node->getOperand(1).get());
88-
assert(Elements && "Invalid Metadata type on root signature");
112+
// Loop through the Root Elements of the root signature.
113+
for (unsigned int Eid = 0; Eid < RS->getNumOperands(); Eid++) {
89114

90-
for (unsigned int Eid = 0; Eid < Elements->getNumOperands(); Eid++) {
91-
MDNode *Element = cast<MDNode>(Elements->getOperand(Eid));
92-
assert(Element && "Invalid Metadata type on root element");
115+
MDNode *Element = dyn_cast<MDNode>(RS->getOperand(Eid));
116+
if (Element == nullptr)
117+
return reportError("Missing Root Element Metadata Node.");
93118

94119
HasError = HasError || parseRootSignatureElement(this, Element);
95120
}
96121
}
97122
return HasError;
98123
}
99124

100-
AnalysisKey RootSignatureAnalysis::Key;
101-
102-
ModuleRootSignature RootSignatureAnalysis::run(Module &M,
103-
ModuleAnalysisManager &AM) {
104-
ModuleRootSignature MRSI;
125+
ModuleRootSignature ModuleRootSignature::analyzeModule(Module &M) {
126+
ModuleRootSignature MRS;
105127

106128
NamedMDNode *RootSignatureNode = M.getNamedMetadata("dx.rootsignatures");
107129
if (RootSignatureNode) {
108-
MRSI.parse(1, RootSignatureNode);
130+
if (MRS.parse(1, RootSignatureNode))
131+
llvm_unreachable("Invalid Root Signature Metadata.");
109132
}
110133

111-
return MRSI;
134+
return MRS;
135+
}
136+
137+
AnalysisKey RootSignatureAnalysis::Key;
138+
139+
ModuleRootSignature RootSignatureAnalysis::run(Module &M,
140+
ModuleAnalysisManager &AM) {
141+
return ModuleRootSignature::analyzeModule(M);
112142
}
113143

114144
//===----------------------------------------------------------------------===//
115145
bool RootSignatureAnalysisWrapper::runOnModule(Module &M) {
116-
ModuleRootSignature MRS;
117146

118-
NamedMDNode *RootSignatureNode = M.getNamedMetadata("dx.rootsignatures");
119-
if (RootSignatureNode) {
120-
MRS.parse(1, RootSignatureNode);
121-
this->MRS = MRS;
122-
}
147+
this->MRS = MRS = ModuleRootSignature::analyzeModule(M);
123148

124149
return false;
125150
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ struct ModuleRootSignature {
3636
ModuleRootSignature() = default;
3737

3838
bool parse(int32_t Version, NamedMDNode *Root);
39+
40+
static ModuleRootSignature analyzeModule(Module &M);
3941
};
4042

4143
class RootSignatureAnalysis : public AnalysisInfoMixin<RootSignatureAnalysis> {

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Error.ll‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
; RUN: not llc %s --filetype=obj -o - 2>&1 | FileCheck %s
2+
3+
target triple = "dxil-unknown-shadermodel6.0-compute"
4+
5+
; CHECK: LLVM ERROR: Invalid format for Root Signature Definition. Pairs of function, root signature expected.
6+
7+
8+
define void @main() #0 {
9+
entry:
10+
ret void
11+
}
12+
13+
attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
14+
15+
16+
!dx.rootsignatures = !{!1} ; list of function/root signature pairs
17+
!1= !{ !"RootFlags" } ; function, root signature

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags-Error.ll‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
; RUN: not llc %s --filetype=obj -o - 2>&1 | FileCheck %s
2+
3+
target triple = "dxil-unknown-shadermodel6.0-compute"
4+
5+
; CHECK: LLVM ERROR: Invalid Root Element: NOTRootFlags
6+
7+
8+
define void @main() #0 {
9+
entry:
10+
ret void
11+
}
12+
13+
attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
14+
15+
16+
!dx.rootsignatures = !{!2} ; list of function/root signature pairs
17+
!2 = !{ ptr @main, !3 } ; function, root signature
18+
!3 = !{ !4 } ; list of root signature elements
19+
!4 = !{ !"NOTRootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags-Validation-Error.ll‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
; RUN: not llc %s --filetype=obj -o - 2>&1 | FileCheck %s
2+
3+
target triple = "dxil-unknown-shadermodel6.0-compute"
4+
5+
; CHECK: LLVM ERROR: Invalid flag value for RootFlag
6+
7+
8+
define void @main() #0 {
9+
entry:
10+
ret void
11+
}
12+
13+
attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
14+
15+
16+
!dx.rootsignatures = !{!2} ; list of function/root signature pairs
17+
!2 = !{ ptr @main, !3 } ; function, root signature
18+
!3 = !{ !4 } ; list of root signature elements
19+
!4 = !{ !"RootFlags", i32 2147487744 } ; 1 = allow_input_assembler_input_layout

‎llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootElement-Error.ll‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
; RUN: not llc %s --filetype=obj -o - 2>&1 | FileCheck %s
2+
3+
target triple = "dxil-unknown-shadermodel6.0-compute"
4+
5+
; CHECK: LLVM ERROR: Missing Root Element Metadata Node.
6+
7+
8+
define void @main() #0 {
9+
entry:
10+
ret void
11+
}
12+
13+
attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
14+
15+
16+
!dx.rootsignatures = !{!2} ; list of function/root signature pairs
17+
!2 = !{ ptr @main, !3 } ; function, root signature
18+
!3 = !{ !"NOTRootElements" } ; list of root signature elements

0 commit comments

Comments
 (0)