From b7d1c195c577fba46ff2a7025f344802dec12c04 Mon Sep 17 00:00:00 2001 From: Helena Kotas Date: Fri, 2 Nov 2018 23:52:32 -0700 Subject: [PATCH] Add root signature flags (#1663) * Add root signature flags - Add LOCAL_ROOT_SIGNATURE flag in RootFlags - Add DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS flag in DescriptorTable - New DxilRootSignatureCompilationFlags argument on Compile/ParseRootSignature to specify whether the root signature is local or global. * Fix root signature flag issues, add cmd test for root sig from define --- .../dxc/DxilRootSignature/DxilRootSignature.h | 11 ++++- tools/clang/include/clang/Parse/ParseHLSL.h | 5 ++- tools/clang/lib/CodeGen/CGHLSLMS.cpp | 16 +++++--- tools/clang/lib/Frontend/FrontendActions.cpp | 3 +- tools/clang/lib/Parse/HLSLRootSignature.cpp | 40 +++++++++++++++---- tools/clang/lib/Parse/HLSLRootSignature.h | 4 ++ .../CodeGenHLSL/quick-test/subobjects.hlsl | 2 +- tools/clang/unittests/HLSL/FunctionTest.cpp | 4 +- utils/hct/hcttestcmds.cmd | 20 ++++++++++ 9 files changed, 85 insertions(+), 20 deletions(-) diff --git a/include/dxc/DxilRootSignature/DxilRootSignature.h b/include/dxc/DxilRootSignature/DxilRootSignature.h index 0ec87c5d5..77c1a6ca6 100644 --- a/include/dxc/DxilRootSignature/DxilRootSignature.h +++ b/include/dxc/DxilRootSignature/DxilRootSignature.h @@ -69,7 +69,8 @@ enum class DxilDescriptorRangeFlags : unsigned { DataVolatile = 0x2, DataStaticWhileSetAtExecute = 0x4, DataStatic = 0x8, - ValidFlags = 0xf, + DescriptorsStaticKeepingBufferBoundsChecks = 0x10000, + ValidFlags = 0x1000f, ValidSamplerFlags = DescriptorsVolatile }; enum class DxilDescriptorRangeType : unsigned { @@ -90,6 +91,11 @@ enum class DxilRootSignatureVersion { Version_1_0 = 1, Version_1_1 = 2 }; +enum class DxilRootSignatureCompilationFlags { + None = 0x0, + LocalRootSignature = 0x1, + GlobalRootSignature = 0x2, +}; enum class DxilRootSignatureFlags : uint32_t { None = 0, AllowInputAssemblerInputLayout = 0x1, @@ -99,8 +105,9 @@ enum class DxilRootSignatureFlags : uint32_t { DenyGeometryShaderRootAccess = 0x10, DenyPixelShaderRootAccess = 0x20, AllowStreamOutput = 0x40, + LocalRootSignature = 0x80, AllowLowTierReservedHwCbLimit = 0x80000000, - ValidFlags = 0x8000007f + ValidFlags = 0x800000ff }; enum class DxilRootParameterType { DescriptorTable = 0, diff --git a/tools/clang/include/clang/Parse/ParseHLSL.h b/tools/clang/include/clang/Parse/ParseHLSL.h index 27d82fd2f..641982425 100644 --- a/tools/clang/include/clang/Parse/ParseHLSL.h +++ b/tools/clang/include/clang/Parse/ParseHLSL.h @@ -19,6 +19,7 @@ class raw_ostream; namespace hlsl { enum class DxilRootSignatureVersion; +enum class DxilRootSignatureCompilationFlags; struct DxilVersionedRootSignatureDesc; class RootSignatureHandle; } @@ -28,6 +29,7 @@ class DiagnosticsEngine; bool ParseHLSLRootSignature(_In_count_(Len) const char *pData, unsigned Len, hlsl::DxilRootSignatureVersion Ver, + hlsl::DxilRootSignatureCompilationFlags Flags, hlsl::DxilVersionedRootSignatureDesc **ppDesc, clang::SourceLocation Loc, clang::DiagnosticsEngine &Diags); @@ -35,8 +37,9 @@ void ReportHLSLRootSigError(clang::DiagnosticsEngine &Diags, clang::SourceLocation Loc, _In_count_(Len) const char *pData, unsigned Len); void CompileRootSignature(StringRef rootSigStr, DiagnosticsEngine &Diags, - SourceLocation SLoc, + SourceLocation SLoc, hlsl::DxilRootSignatureVersion rootSigVer, + hlsl::DxilRootSignatureCompilationFlags flags, hlsl::RootSignatureHandle *pRootSigHandle); } diff --git a/tools/clang/lib/CodeGen/CGHLSLMS.cpp b/tools/clang/lib/CodeGen/CGHLSLMS.cpp index 757646ac5..7da822cd2 100644 --- a/tools/clang/lib/CodeGen/CGHLSLMS.cpp +++ b/tools/clang/lib/CodeGen/CGHLSLMS.cpp @@ -322,13 +322,14 @@ public: void clang::CompileRootSignature( StringRef rootSigStr, DiagnosticsEngine &Diags, SourceLocation SLoc, hlsl::DxilRootSignatureVersion rootSigVer, + hlsl::DxilRootSignatureCompilationFlags flags, hlsl::RootSignatureHandle *pRootSigHandle) { std::string OSStr; llvm::raw_string_ostream OS(OSStr); hlsl::DxilVersionedRootSignatureDesc *D = nullptr; if (ParseHLSLRootSignature(rootSigStr.data(), rootSigStr.size(), rootSigVer, - &D, SLoc, Diags)) { + flags, &D, SLoc, Diags)) { CComPtr pSignature; CComPtr pErrors; hlsl::SerializeRootSignature(D, &pSignature, &pErrors, false); @@ -2464,6 +2465,7 @@ void CGMSHLSLRuntime::CreateSubobject(DXIL::SubobjectKind kind, const StringRef m_pHLModule->ResetSubobjects(subobjects); } + DxilRootSignatureCompilationFlags flags = DxilRootSignatureCompilationFlags::GlobalRootSignature; switch (kind) { case DXIL::SubobjectKind::StateObjectConfig: { uint32_t flags; @@ -2473,15 +2475,17 @@ void CGMSHLSLRuntime::CreateSubobject(DXIL::SubobjectKind kind, const StringRef } break; } - case DXIL::SubobjectKind::GlobalRootSignature: - case DXIL::SubobjectKind::LocalRootSignature: { + case DXIL::SubobjectKind::LocalRootSignature: + flags = DxilRootSignatureCompilationFlags::LocalRootSignature; + __fallthrough; + case DXIL::SubobjectKind::GlobalRootSignature: { DXASSERT_NOMSG(argCount == 1); StringRef signature; if (!GetAsConstantString(args[0], &signature, true)) return; RootSignatureHandle RootSigHandle; - CompileRootSignature(signature, CGM.getDiags(), args[0]->getLocStart(), rootSigVer, &RootSigHandle); + CompileRootSignature(signature, CGM.getDiags(), args[0]->getLocStart(), rootSigVer, flags, &RootSigHandle); if (!RootSigHandle.IsEmpty()) { RootSigHandle.EnsureSerializedAvailable(); @@ -5013,7 +5017,7 @@ void CGMSHLSLRuntime::FinishCodeGen() { RootSignatureHandle RootSigHandle; CompileRootSignature(customRootSig.RootSignature, Diags, SourceLocation::getFromRawEncoding(customRootSig.EncodedSourceLocation), - rootSigVer, &RootSigHandle); + rootSigVer, DxilRootSignatureCompilationFlags::GlobalRootSignature, &RootSigHandle); if (!RootSigHandle.IsEmpty()) { RootSigHandle.EnsureSerializedAvailable(); m_pHLModule->SetSerializedRootSignature( @@ -6970,7 +6974,7 @@ void CGMSHLSLRuntime::EmitHLSLRootSignature(CodeGenFunction &CGF, DiagnosticsEngine &Diags = CGF.getContext().getDiagnostics(); SourceLocation SLoc = RSA->getLocation(); RootSignatureHandle RootSigHandle; - clang::CompileRootSignature(StrRef, Diags, SLoc, rootSigVer, &RootSigHandle); + clang::CompileRootSignature(StrRef, Diags, SLoc, rootSigVer, DxilRootSignatureCompilationFlags::GlobalRootSignature, &RootSigHandle); if (!RootSigHandle.IsEmpty()) { RootSigHandle.EnsureSerializedAvailable(); m_pHLModule->SetSerializedRootSignature(RootSigHandle.GetSerializedBytes(), diff --git a/tools/clang/lib/Frontend/FrontendActions.cpp b/tools/clang/lib/Frontend/FrontendActions.cpp index cdc8cf161..e8647eea3 100644 --- a/tools/clang/lib/Frontend/FrontendActions.cpp +++ b/tools/clang/lib/Frontend/FrontendActions.cpp @@ -758,7 +758,8 @@ void HLSLRootSignatureAction::ExecuteAction() { } // Compile the expanded root signature. - clang::CompileRootSignature(rootSigString, Diags, SLoc, rootSigVer, rootSigHandle.get()); + clang::CompileRootSignature(rootSigString, Diags, SLoc, rootSigVer, + hlsl::DxilRootSignatureCompilationFlags::None, rootSigHandle.get()); } std::unique_ptr HLSLRootSignatureAction::takeRootSigHandle() { diff --git a/tools/clang/lib/Parse/HLSLRootSignature.cpp b/tools/clang/lib/Parse/HLSLRootSignature.cpp index 3b48fa6a1..d29b8582d 100644 --- a/tools/clang/lib/Parse/HLSLRootSignature.cpp +++ b/tools/clang/lib/Parse/HLSLRootSignature.cpp @@ -35,6 +35,7 @@ using namespace hlsl; #define ERR_RS_NOT_ALLOWED_FOR_HS_PATCH_CONST 4618 #define ERR_RS_WRONG_ROOT_DESC_FLAG 4619 #define ERR_RS_WRONG_DESC_RANGE_FLAG 4620 +#define ERR_RS_LOCAL_FLAG_ON_GLOBAL 4621 #define ERR_RS_BAD_FLOAT 5000 // Non-throwing new @@ -43,6 +44,7 @@ using namespace hlsl; DEFINE_ENUM_FLAG_OPERATORS(DxilRootSignatureFlags) DEFINE_ENUM_FLAG_OPERATORS(DxilRootDescriptorFlags) DEFINE_ENUM_FLAG_OPERATORS(DxilDescriptorRangeFlags) +DEFINE_ENUM_FLAG_OPERATORS(DxilRootSignatureCompilationFlags) RootSignatureTokenizer::RootSignatureTokenizer(const char *pStr, size_t len) : m_pStrPos(pStr) @@ -288,8 +290,9 @@ void RootSignatureTokenizer::ReadNextToken(uint32_t BufferIdx) KW(DESCRIPTORS_VOLATILE) || KW(DATA_VOLATILE) || KW(DATA_STATIC) || - KW(DATA_STATIC_WHILE_SET_AT_EXECUTE); - break; + KW(DATA_STATIC_WHILE_SET_AT_EXECUTE) || + KW(DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS); + break; case 'F': bKW = KW(flags) || @@ -332,6 +335,10 @@ void RootSignatureTokenizer::ReadNextToken(uint32_t BufferIdx) KW(FILTER_MAXIMUM_ANISOTROPIC); break; + case 'L': + bKW = KW(LOCAL_ROOT_SIGNATURE); + break; + case 'M': bKW = KW(maxAnisotropy) || KW(mipLODBias) || KW(minLOD) || KW(maxLOD); break; @@ -476,14 +483,18 @@ bool RootSignatureTokenizer::IsAlpha(char c) const RootSignatureParser::RootSignatureParser( RootSignatureTokenizer *pTokenizer, DxilRootSignatureVersion DefaultVersion, - llvm::raw_ostream &OS) - : m_pTokenizer(pTokenizer), m_Version(DefaultVersion), m_OS(OS) {} + DxilRootSignatureCompilationFlags CompilationFlags, llvm::raw_ostream &OS) + : m_pTokenizer(pTokenizer), m_Version(DefaultVersion), m_CompilationFlags(CompilationFlags), m_OS(OS) {} HRESULT RootSignatureParser::Parse(DxilVersionedRootSignatureDesc **ppRootSignature) { HRESULT hr = S_OK; DxilVersionedRootSignatureDesc *pRS = NULL; + DXASSERT(!((bool)(m_CompilationFlags & DxilRootSignatureCompilationFlags::GlobalRootSignature) && + (bool)(m_CompilationFlags & DxilRootSignatureCompilationFlags::LocalRootSignature)), + "global and local cannot be both set"); + if(ppRootSignature != NULL) { IFC(ParseRootSignature(&pRS)); @@ -631,6 +642,10 @@ HRESULT RootSignatureParser::ParseRootSignature(DxilVersionedRootSignatureDesc * pRS->Desc_1_1.pStaticSamplers = pStaticSamplers; } + // Set local signature flag if not already on + if ((bool)(m_CompilationFlags & DxilRootSignatureCompilationFlags::LocalRootSignature)) + pRS->Desc_1_1.Flags |= DxilRootSignatureFlags::LocalRootSignature; + // Down-convert root signature to the right version, if needed. if(pRS->Version != m_Version) { @@ -664,6 +679,7 @@ HRESULT RootSignatureParser::ParseRootSignatureFlags(DxilRootSignatureFlags & Fl // DENY_GEOMETRY_SHADER_ROOT_ACCESS // DENY_PIXEL_SHADER_ROOT_ACCESS // ALLOW_STREAM_OUTPUT + // LOCAL_ROOT_SIGNATURE HRESULT hr = S_OK; TokenType Token; @@ -711,6 +727,11 @@ HRESULT RootSignatureParser::ParseRootSignatureFlags(DxilRootSignatureFlags & Fl case TokenType::ALLOW_STREAM_OUTPUT: Flags |= DxilRootSignatureFlags::AllowStreamOutput; break; + case TokenType::LOCAL_ROOT_SIGNATURE: + if ((bool)(m_CompilationFlags & DxilRootSignatureCompilationFlags::GlobalRootSignature)) + IFC(Error(ERR_RS_LOCAL_FLAG_ON_GLOBAL, "LOCAL_ROOT_SIGNATURE flag used in global root signature")); + Flags |= DxilRootSignatureFlags::LocalRootSignature; + break; default: IFC(Error(ERR_RS_UNEXPECTED_TOKEN, "Expected a root signature flag value, found: '%s'", Token.GetStr())); } @@ -1166,7 +1187,7 @@ Cleanup: HRESULT RootSignatureParser::ParseDescRangeFlags(DxilDescriptorRangeType, DxilDescriptorRangeFlags & Flags) { - // flags=DESCRIPTORS_VOLATILE | DATA_VOLATILE | DATA_STATIC | DATA_STATIC_WHILE_SET_AT_EXECUTE + // flags=DESCRIPTORS_VOLATILE | DATA_VOLATILE | DATA_STATIC | DATA_STATIC_WHILE_SET_AT_EXECUTE | DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS HRESULT hr = S_OK; TokenType Token; @@ -1210,6 +1231,9 @@ HRESULT RootSignatureParser::ParseDescRangeFlags(DxilDescriptorRangeType, case TokenType::DATA_STATIC_WHILE_SET_AT_EXECUTE: Flags |= DxilDescriptorRangeFlags::DataStaticWhileSetAtExecute; break; + case TokenType::DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS: + Flags |= DxilDescriptorRangeFlags::DescriptorsStaticKeepingBufferBoundsChecks; + break; default: IFC(Error(ERR_RS_UNEXPECTED_TOKEN, "Expected a descriptor range flag value, found: '%s'", Token.GetStr())); } @@ -1638,13 +1662,13 @@ Cleanup: _Use_decl_annotations_ bool clang::ParseHLSLRootSignature( const char *pData, unsigned Len, hlsl::DxilRootSignatureVersion Ver, - hlsl::DxilVersionedRootSignatureDesc **ppDesc, SourceLocation Loc, - clang::DiagnosticsEngine &Diags) { + hlsl::DxilRootSignatureCompilationFlags Flags, hlsl::DxilVersionedRootSignatureDesc **ppDesc, + SourceLocation Loc, clang::DiagnosticsEngine &Diags) { *ppDesc = nullptr; std::string OSStr; llvm::raw_string_ostream OS(OSStr); hlsl::RootSignatureTokenizer RST(pData, Len); - hlsl::RootSignatureParser RSP(&RST, Ver, OS); + hlsl::RootSignatureParser RSP(&RST, Ver, Flags, OS); hlsl::DxilVersionedRootSignatureDesc *D = nullptr; if (SUCCEEDED(RSP.Parse(&D))) { *ppDesc = D; diff --git a/tools/clang/lib/Parse/HLSLRootSignature.h b/tools/clang/lib/Parse/HLSLRootSignature.h index bc19643a5..a1431ed83 100644 --- a/tools/clang/lib/Parse/HLSLRootSignature.h +++ b/tools/clang/lib/Parse/HLSLRootSignature.h @@ -59,6 +59,7 @@ public: DATA_VOLATILE, DATA_STATIC, DATA_STATIC_WHILE_SET_AT_EXECUTE, + DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS, // Visibility visibility, @@ -78,6 +79,7 @@ public: DENY_GEOMETRY_SHADER_ROOT_ACCESS, DENY_PIXEL_SHADER_ROOT_ACCESS, ALLOW_STREAM_OUTPUT, + LOCAL_ROOT_SIGNATURE, // Filter filter, @@ -224,6 +226,7 @@ class RootSignatureParser public: RootSignatureParser(RootSignatureTokenizer *pTokenizer, DxilRootSignatureVersion DefaultVersion, + DxilRootSignatureCompilationFlags Flags, llvm::raw_ostream &OS); HRESULT Parse(DxilVersionedRootSignatureDesc **ppRootSignature); @@ -233,6 +236,7 @@ private: RootSignatureTokenizer *m_pTokenizer; DxilRootSignatureVersion m_Version; + DxilRootSignatureCompilationFlags m_CompilationFlags; llvm::raw_ostream &m_OS; HRESULT GetAndMatchToken(TokenType & Token, TokenType::Type Type); diff --git a/tools/clang/test/CodeGenHLSL/quick-test/subobjects.hlsl b/tools/clang/test/CodeGenHLSL/quick-test/subobjects.hlsl index dd4c96940..c142cb72c 100644 --- a/tools/clang/test/CodeGenHLSL/quick-test/subobjects.hlsl +++ b/tools/clang/test/CodeGenHLSL/quick-test/subobjects.hlsl @@ -11,7 +11,7 @@ GlobalRootSignature grs = {"CBV(b0)"}; StateObjectConfig soc = { STATE_OBJECT_FLAGS_ALLOW_LOCAL_DEPENDENCIES_ON_EXTERNAL_DEFINITONS }; -LocalRootSignature lrs = {"UAV(u0, visibility = SHADER_VISIBILITY_GEOMETRY)"}; +LocalRootSignature lrs = {"UAV(u0, visibility = SHADER_VISIBILITY_GEOMETRY), RootFlags(LOCAL_ROOT_SIGNATURE)"}; SubobjectToExportsAssociation sea = { "grs", "a;b;foo;c" }; SubobjectToExportsAssociation sea2 = { "grs", ";" }; RaytracingShaderConfig rsc = { 128, 64 }; diff --git a/tools/clang/unittests/HLSL/FunctionTest.cpp b/tools/clang/unittests/HLSL/FunctionTest.cpp index 96c4ae953..bcb3edeb2 100644 --- a/tools/clang/unittests/HLSL/FunctionTest.cpp +++ b/tools/clang/unittests/HLSL/FunctionTest.cpp @@ -183,6 +183,7 @@ TEST_F(FunctionTest, ParseRootSignature) { TestHLSLRootSignatureCase("RootFlags( 20 )", E_FAIL); TestHLSLRootSignatureCase("RootFlags(ALLOW_INPUT_ASSEMBLER_INPUT_LAYOUT)", S_OK); TestHLSLRootSignatureCase("RootFlags(ALLOW_STREAM_OUTPUT)", S_OK); + TestHLSLRootSignatureCase("RootFlags(LOCAL_ROOT_SIGNATURE)", E_FAIL); TestHLSLRootSignatureCase("RootFlags( LLOW_INPUT_ASSEMBLER_INPUT_LAYOUT)", E_FAIL); TestHLSLRootSignatureCase(" RootFlags ( ALLOW_INPUT_ASSEMBLER_INPUT_LAYOUT ) ", S_OK); TestHLSLRootSignatureCase("RootFlags(ALLOW_INPUT_ASSEMBLER_INPUT_LAYOUT | DENY_VERTEX_SHADER_ROOT_ACCESS | DENY_HULL_SHADER_ROOT_ACCESS | DENY_DOMAIN_SHADER_ROOT_ACCESS | DENY_GEOMETRY_SHADER_ROOT_ACCESS | DENY_PIXEL_SHADER_ROOT_ACCESS)", S_OK); @@ -307,7 +308,8 @@ TEST_F(FunctionTest, ParseRootSignature) { TestHLSLRootSignature11Case("DescriptorTable(UAV(u2, flags = DESCRIPTORS_VOLATILE | DATA_STATIC_WHILE_SET_AT_EXECUTE))", S_OK); TestHLSLRootSignature11Case("DescriptorTable(Sampler(s2, flags = DESCRIPTORS_VOLATILE))", S_OK); TestHLSLRootSignature11Case("DescriptorTable(UAV(u2, flags = DESCRIPTORS_VOLATILE | DATA_STATIC_WHILE_SET_AT_EXECUTE, offset =17))", S_OK); - + TestHLSLRootSignature11Case("DescriptorTable(UAV(u2, flags = DESCRIPTORS_STATIC_KEEPING_BUFFER_BOUNDS_CHECKS))", S_OK); + TestHLSLRootSignature10Case("DescriptorTable(CBV(b2, flags = 0))", E_FAIL); TestHLSLRootSignature10Case("DescriptorTable(SRV(t2, flags = DESCRIPTORS_VOLATILE))", E_FAIL); TestHLSLRootSignature10Case("DescriptorTable(UAV(u2, flags = DATA_VOLATILE))", E_FAIL); diff --git a/utils/hct/hcttestcmds.cmd b/utils/hct/hcttestcmds.cmd index 1e37808cc..61ec3d86c 100644 --- a/utils/hct/hcttestcmds.cmd +++ b/utils/hct/hcttestcmds.cmd @@ -159,6 +159,22 @@ if %errorlevel% equ 0 ( exit /b 1 ) +echo #define main "CBV(b0)"> test-global-rs.hlsl +dxc -T rootsig_1_1 test-global-rs.hlsl -rootsig-define main -Fo test-global-rs.cso 1>nul +if %errorlevel% neq 0 ( + echo Failed to compile rootsig_1_1 from define + call :cleanup 2>nul + exit /b 1 +) + +echo #define main "CBV(b0), RootFlags(LOCAL_ROOT_SIGNATURE)"> test-local-rs.hlsl +dxc -T rootsig_1_1 test-local-rs.hlsl -rootsig-define main -Fo test-local-rs.cso 1>nul +if %errorlevel% neq 0 ( + echo Failed to compile rootsig_1_1 from define with LOCAL_ROOT_SIGNATURE + call :cleanup 2>nul + exit /b 1 +) + dxc.exe /T ps_6_0 %script_dir%\smoke.hlsl /HV 2016 1>nul if %errorlevel% neq 0 ( echo Failed to compile with HLSL version 2016 @@ -655,6 +671,10 @@ rem SPIR-V Change Ends del %CD%\lib_res_match.dxbc del %CD%\lib_entry4.dxbc del %CD%\res_match_entry.dxbc +del %CD%\test-global-rs.hlsl +del %CD%\test-local-rs.hlsl +del %CD%\test-global-rs.cso +del %CD%\test-local-rs.cso exit /b 0