-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[DXIL] Add support for root signature flag element in DXContainer #123147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
8adb678
ba78f21
0a54559
557075f
e0d3dcd
80587dd
be3764d
7582ca6
6aaa0a5
916b2f1
d9bce0a
e7676ed
a0cee57
1e7a1fe
0ed658a
932062e
628937c
1378c9f
e3206c9
f93d42d
25e3f37
751cbdc
44532d6
ca21878
987901c
0fbe900
b771aea
aabdfe7
4f6f941
a7f7784
bf3b2e0
16b4d03
e043370
57bf935
1f8c0a5
0905b83
77e8544
1351fb0
09e645a
5a44b62
d1a79b3
9f8e512
5c7ed7e
93f7c4c
5aac761
47b01f7
486ab88
852ac25
74f7226
7a82fa9
c67d039
e80ef33
ef1ce8a
b7f2716
83b0979
b175b65
01b49a7
7bba9d3
2809c2f
023dcb8
aedb446
39e60c0
3b135c6
ae3d03f
3d19f8b
5a3be7c
f64c608
a5a2093
5b3fedc
605f225
e4bca2b
825673f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -20,6 +20,7 @@ | |||||||||||||||
#include "llvm/IR/DiagnosticInfo.h" | ||||||||||||||||
#include "llvm/IR/Function.h" | ||||||||||||||||
#include "llvm/IR/LLVMContext.h" | ||||||||||||||||
#include "llvm/IR/Metadata.h" | ||||||||||||||||
#include "llvm/IR/Module.h" | ||||||||||||||||
#include "llvm/InitializePasses.h" | ||||||||||||||||
#include "llvm/Pass.h" | ||||||||||||||||
|
@@ -56,16 +57,12 @@ static bool parseRootSignatureElement(LLVMContext *Ctx, | |||||||||||||||
|
||||||||||||||||
RootSignatureElementKind ElementKind = | ||||||||||||||||
StringSwitch<RootSignatureElementKind>(ElementText->getString()) | ||||||||||||||||
.Case("RootFlags", RootSignatureElementKind::RootFlags) | ||||||||||||||||
.Default(RootSignatureElementKind::None); | ||||||||||||||||
.Case("RootFlags", RootSignatureElementKind::RootFlags); | ||||||||||||||||
|
||||||||||||||||
switch (ElementKind) { | ||||||||||||||||
|
||||||||||||||||
case RootSignatureElementKind::RootFlags: | ||||||||||||||||
return parseRootFlags(Ctx, MRS, Element); | ||||||||||||||||
default: | ||||||||||||||||
return reportError(Ctx, | ||||||||||||||||
"Invalid Root Element: " + ElementText->getString()); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
return true; | ||||||||||||||||
damyanp marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||
|
@@ -95,8 +92,14 @@ static bool parse(LLVMContext *Ctx, ModuleRootSignature *MRS, NamedMDNode *Root, | |||||||||||||||
continue; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
const MDOperand &FunctionPointerMdNode = Node->getOperand(0); | ||||||||||||||||
if (FunctionPointerMdNode == nullptr) { | ||||||||||||||||
// Function was pruned during compilation. | ||||||||||||||||
continue; | ||||||||||||||||
} | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you construct a test case that includes this? Is this something that can actually happen - a function is referenced by some metadata, and the function is removed but not the metadata that references it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I found this error when running test: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that normal, or is this an indication of a bug somewhere where we also need to strip the corresponding metadata when a function is pruned? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems expected, not used functions will be removed, since we record function pointers in the metadata, if the function is removed the pointer will be null. @bogner please correct me. I think we should clean it, but we might need to do it in another pass. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should be able to get here without hand-written invalid IR. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||
|
||||||||||||||||
ValueAsMetadata *VAM = | ||||||||||||||||
llvm::dyn_cast<ValueAsMetadata>(Node->getOperand(0).get()); | ||||||||||||||||
llvm::dyn_cast<ValueAsMetadata>(FunctionPointerMdNode.get()); | ||||||||||||||||
if (VAM == nullptr) { | ||||||||||||||||
HasError = | ||||||||||||||||
reportError(Ctx, "First element of root signature is not a value"); | ||||||||||||||||
|
@@ -140,24 +143,26 @@ static bool validate(LLVMContext *Ctx, ModuleRootSignature *MRS) { | |||||||||||||||
return false; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
std::optional<ModuleRootSignature> | ||||||||||||||||
ModuleRootSignature::analyzeModule(Module &M, ModuleMetadataInfo MMI) { | ||||||||||||||||
if (MMI.ShaderProfile == Triple::Library) | ||||||||||||||||
return std::nullopt; | ||||||||||||||||
static const Function *getEntryFunction(Module &M, ModuleMetadataInfo MMI) { | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't need to copy or modify the MMI here.
Suggested change
|
||||||||||||||||
|
||||||||||||||||
LLVMContext *Ctx = &M.getContext(); | ||||||||||||||||
|
||||||||||||||||
if (MMI.EntryPropertyVec.size() != 1) { | ||||||||||||||||
reportError(Ctx, "More than one entry function defined."); | ||||||||||||||||
return std::nullopt; | ||||||||||||||||
return nullptr; | ||||||||||||||||
} | ||||||||||||||||
return MMI.EntryPropertyVec[0].Entry; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
std::optional<ModuleRootSignature> | ||||||||||||||||
ModuleRootSignature::analyzeModule(Module &M, const Function *F) { | ||||||||||||||||
|
||||||||||||||||
LLVMContext *Ctx = &M.getContext(); | ||||||||||||||||
|
||||||||||||||||
ModuleRootSignature MRS; | ||||||||||||||||
const Function *EntryFunction = MMI.EntryPropertyVec[0].Entry; | ||||||||||||||||
|
||||||||||||||||
NamedMDNode *RootSignatureNode = M.getNamedMetadata("dx.rootsignatures"); | ||||||||||||||||
damyanp marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||
if (RootSignatureNode == nullptr || | ||||||||||||||||
parse(Ctx, &MRS, RootSignatureNode, EntryFunction) || validate(Ctx, &MRS)) | ||||||||||||||||
if (RootSignatureNode == nullptr || parse(Ctx, &MRS, RootSignatureNode, F) || | ||||||||||||||||
validate(Ctx, &MRS)) | ||||||||||||||||
return std::nullopt; | ||||||||||||||||
|
||||||||||||||||
return MRS; | ||||||||||||||||
|
@@ -168,15 +173,18 @@ AnalysisKey RootSignatureAnalysis::Key; | |||||||||||||||
std::optional<ModuleRootSignature> | ||||||||||||||||
RootSignatureAnalysis::run(Module &M, ModuleAnalysisManager &AM) { | ||||||||||||||||
ModuleMetadataInfo MMI = AM.getResult<DXILMetadataAnalysis>(M); | ||||||||||||||||
return ModuleRootSignature::analyzeModule(M, MMI); | ||||||||||||||||
if (MMI.ShaderProfile == Triple::Library) | ||||||||||||||||
return std::nullopt; | ||||||||||||||||
return ModuleRootSignature::analyzeModule(M, getEntryFunction(M, MMI)); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
//===----------------------------------------------------------------------===// | ||||||||||||||||
bool RootSignatureAnalysisWrapper::runOnModule(Module &M) { | ||||||||||||||||
|
||||||||||||||||
dxil::ModuleMetadataInfo &MMI = | ||||||||||||||||
getAnalysis<DXILMetadataAnalysisWrapperPass>().getModuleMetadata(); | ||||||||||||||||
MRS = ModuleRootSignature::analyzeModule(M, MMI); | ||||||||||||||||
if (MMI.ShaderProfile == Triple::Library) | ||||||||||||||||
return false; | ||||||||||||||||
MRS = ModuleRootSignature::analyzeModule(M, getEntryFunction(M, MMI)); | ||||||||||||||||
return false; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,12 +22,12 @@ | |
namespace llvm { | ||
namespace dxil { | ||
|
||
enum class RootSignatureElementKind { None = 0, RootFlags = 1 }; | ||
enum class RootSignatureElementKind { RootFlags = 1 }; | ||
|
||
struct ModuleRootSignature { | ||
uint32_t Flags = 0; | ||
static std::optional<ModuleRootSignature> | ||
analyzeModule(Module &M, ModuleMetadataInfo MMI); | ||
static std::optional<ModuleRootSignature> analyzeModule(Module &M, | ||
const Function *F); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't really need to be public API like this - better to just have a |
||
}; | ||
|
||
class RootSignatureAnalysis : public AnalysisInfoMixin<RootSignatureAnalysis> { | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
; RUN: not --crash llc %s --filetype=obj -o - | ||
; expected-error@-1: More than one entry function defined | ||
damyanp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
target triple = "dxil-unknown-shadermodel6.0-compute" | ||
|
||
|
||
define void @main() #0 { | ||
entry: | ||
ret void | ||
} | ||
|
||
define void @anotherMain() #1 { | ||
entry: | ||
ret void | ||
} | ||
|
||
attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } | ||
attributes #1 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } | ||
|
||
!dx.rootsignatures = !{!2, !5} ; list of function/root signature pairs | ||
!2 = !{ ptr @main, !3 } ; function, root signature | ||
!3 = !{ !4 } ; list of root signature elements | ||
!4 = !{ !"RootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout | ||
!5 = !{ ptr @anotherMain, !6 } ; function, root signature | ||
!6 = !{ !7 } ; list of root signature elements | ||
!7 = !{ !"RootFlags", i32 2 } ; 1 = allow_input_assembler_input_layout |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this test trying to cover? There's only one entry function here since |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
; RUN: llc %s --filetype=obj -o - | obj2yaml | FileCheck %s | ||
|
||
target triple = "dxil-unknown-shadermodel6.0-compute" | ||
|
||
|
||
define void @main() { | ||
entry: | ||
ret void | ||
} | ||
|
||
define void @anotherMain() #0 { | ||
entry: | ||
ret void | ||
} | ||
|
||
attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } | ||
|
||
!dx.rootsignatures = !{!2, !5} ; list of function/root signature pairs | ||
!2 = !{ ptr @main, !3 } ; function, root signature | ||
!3 = !{ !4 } ; list of root signature elements | ||
!4 = !{ !"RootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout | ||
!5 = !{ ptr @anotherMain, !6 } ; function, root signature | ||
!6 = !{ !7 } ; list of root signature elements | ||
!7 = !{ !"RootFlags", i32 2 } ; 1 = allow_input_assembler_input_layout | ||
|
||
|
||
; CHECK: - Name: RTS0 | ||
; CHECK-NEXT: Size: 24 | ||
; CHECK-NEXT: RootSignature: | ||
; CHECK-NEXT: Version: 2 | ||
; CHECK-NEXT: NumParameters: 0 | ||
; CHECK-NEXT: RootParametersOffset: 0 | ||
; CHECK-NEXT: NumStaticSamplers: 0 | ||
; CHECK-NEXT: StaticSamplersOffset: 0 | ||
; CHECK-NEXT: DenyVertexShaderRootAccess: true |
Uh oh!
There was an error while loading. Please reload this page.