-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[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 53 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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -9,9 +9,9 @@ | |||||
| #include "llvm/Object/DXContainer.h" | ||||||
| #include "llvm/BinaryFormat/DXContainer.h" | ||||||
| #include "llvm/Object/Error.h" | ||||||
| #include "llvm/Support/FormatVariadic.h" | ||||||
| #include "llvm/Support/Alignment.h" | ||||||
| #include "llvm/Support/Endian.h" | ||||||
| #include "llvm/Support/FormatVariadic.h" | ||||||
bogner marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
|
|
||||||
| using namespace llvm; | ||||||
| using namespace llvm::object; | ||||||
|
|
@@ -254,11 +254,9 @@ Error DirectX::RootSignature::parse(StringRef Data) { | |||||
| support::endian::read<uint32_t, llvm::endianness::little>(Current); | ||||||
| Current += sizeof(uint32_t); | ||||||
|
|
||||||
| Expected<uint32_t> MaybeVersion = | ||||||
| dxbc::RootSignatureValidations::validateVersion(VValue); | ||||||
| if (Error E = MaybeVersion.takeError()) | ||||||
| return E; | ||||||
| Version = MaybeVersion.get(); | ||||||
| if (dxbc::RootSignatureValidations::validateVersion(VValue)) | ||||||
| return make_error<GenericBinaryError>("Invalid Root Signature Version"); | ||||||
|
||||||
| return make_error<GenericBinaryError>("Invalid Root Signature Version"); | |
| return make_error<GenericBinaryError>(Twine("Unsupported root signature version read: " + VValue); |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same feedback as above, we can make this a useful error.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,7 @@ add_llvm_target(DirectXCodeGen | |
| DXILResourceAccess.cpp | ||
| DXILShaderFlags.cpp | ||
| DXILTranslateMetadata.cpp | ||
|
|
||
| DXILRootSignature.cpp | ||
| LINK_COMPONENTS | ||
|
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. Please keep the blank line between the sources and the LINK_COMPONENTS line - it makes this easier to read. |
||
| Analysis | ||
| AsmPrinter | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #include "DXILRootSignature.h" | ||
| #include "DXILShaderFlags.h" | ||
| #include "DirectX.h" | ||
| #include "llvm/ADT/SmallVector.h" | ||
|
|
@@ -23,6 +24,7 @@ | |
| #include "llvm/IR/Module.h" | ||
| #include "llvm/InitializePasses.h" | ||
| #include "llvm/MC/DXContainerPSVInfo.h" | ||
| #include "llvm/MC/DXContainerRootSignature.h" | ||
| #include "llvm/Pass.h" | ||
| #include "llvm/Support/MD5.h" | ||
| #include "llvm/Transforms/Utils/ModuleUtils.h" | ||
|
|
@@ -41,6 +43,7 @@ class DXContainerGlobals : public llvm::ModulePass { | |
| GlobalVariable *buildSignature(Module &M, Signature &Sig, StringRef Name, | ||
| StringRef SectionName); | ||
| void addSignature(Module &M, SmallVector<GlobalValue *> &Globals); | ||
| void addRootSignature(Module &M, SmallVector<GlobalValue *> &Globals); | ||
| void addResourcesForPSV(Module &M, PSVRuntimeInfo &PSV); | ||
| void addPipelineStateValidationInfo(Module &M, | ||
| SmallVector<GlobalValue *> &Globals); | ||
|
|
@@ -60,6 +63,7 @@ class DXContainerGlobals : public llvm::ModulePass { | |
| void getAnalysisUsage(AnalysisUsage &AU) const override { | ||
| AU.setPreservesAll(); | ||
| AU.addRequired<ShaderFlagsAnalysisWrapper>(); | ||
| AU.addRequired<RootSignatureAnalysisWrapper>(); | ||
| AU.addRequired<DXILMetadataAnalysisWrapperPass>(); | ||
| AU.addRequired<DXILResourceTypeWrapperPass>(); | ||
| AU.addRequired<DXILResourceBindingWrapperPass>(); | ||
|
|
@@ -73,6 +77,7 @@ bool DXContainerGlobals::runOnModule(Module &M) { | |
| Globals.push_back(getFeatureFlags(M)); | ||
| Globals.push_back(computeShaderHash(M)); | ||
| addSignature(M, Globals); | ||
| addRootSignature(M, Globals); | ||
| addPipelineStateValidationInfo(M, Globals); | ||
| appendToCompilerUsed(M, Globals); | ||
| return true; | ||
|
|
@@ -144,6 +149,29 @@ void DXContainerGlobals::addSignature(Module &M, | |
| Globals.emplace_back(buildSignature(M, OutputSig, "dx.osg1", "OSG1")); | ||
| } | ||
|
|
||
| void DXContainerGlobals::addRootSignature(Module &M, | ||
| SmallVector<GlobalValue *> &Globals) { | ||
|
|
||
| auto &RSA = getAnalysis<RootSignatureAnalysisWrapper>(); | ||
|
|
||
| if (!RSA.hasRootSignature()) | ||
| return; | ||
|
|
||
| ModuleRootSignature MRS = RSA.getRootSignature(); | ||
|
|
||
| SmallString<256> Data; | ||
| raw_svector_ostream OS(Data); | ||
|
|
||
| RootSignatureHeader RSH; | ||
| RSH.Flags = MRS.Flags; | ||
|
||
|
|
||
| RSH.write(OS); | ||
|
|
||
| Constant *Constant = | ||
| ConstantDataArray::getString(M.getContext(), Data, /*AddNull*/ false); | ||
| Globals.emplace_back(buildContainerGlobal(M, Constant, "dx.rts0", "RTS0")); | ||
| } | ||
|
|
||
| void DXContainerGlobals::addResourcesForPSV(Module &M, PSVRuntimeInfo &PSV) { | ||
| const DXILBindingMap &DBM = | ||
| getAnalysis<DXILResourceBindingWrapperPass>().getBindingMap(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,196 @@ | ||||||
| //===- DXILRootSignature.cpp - DXIL Root Signature helper objects ----===// | ||||||
|
||||||
| //===- DXILRootSignature.cpp - DXIL Root Signature helper objects ----===// | |
| //===- DXILRootSignature.cpp - DXIL Root Signature helper objects -------===// |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should RootSignatureElementKind::None actually be called RootSignatureElementKind::Error? The current name makes me think there is simply no root signature, but the way it's used seems to generally be "the root signature is invalid"
damyanp marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
damyanp marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
damyanp marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
damyanp marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
damyanp marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
damyanp marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
damyanp marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
bogner marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
damyanp marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
damyanp marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
damyanp marked this conversation as resolved.
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be called dxil-root-signature-analysis. The dx- prefix seems to be used in DXILShaderFlags for some reason, but generally we've been consistent with the dxil- prefix.
Uh oh!
There was an error while loading. Please reload this page.