From ce6d208234904bfbac01bfe79ff1275c542e3f54 Mon Sep 17 00:00:00 2001 From: Adam Yang <31109344+adam-yang@users.noreply.github.com> Date: Mon, 1 Nov 2021 18:46:13 -0700 Subject: [PATCH] Added -binding-table-define (#4044) --- include/dxc/Support/HLSLOptions.h | 1 + include/dxc/Support/HLSLOptions.td | 2 + lib/DxcBindingTable/DxcBindingTable.cpp | 22 +-- lib/DxcSupport/HLSLOptions.cpp | 1 + .../include/clang/Frontend/CodeGenOptions.h | 6 + tools/clang/lib/CodeGen/ModuleBuilder.cpp | 21 ++- .../dxil/debug/binding-table.hlsl | 125 ++++++++++++++++++ .../clang/tools/dxcompiler/dxcompilerobj.cpp | 36 ++++- 8 files changed, 202 insertions(+), 12 deletions(-) create mode 100644 tools/clang/test/HLSLFileCheck/dxil/debug/binding-table.hlsl diff --git a/include/dxc/Support/HLSLOptions.h b/include/dxc/Support/HLSLOptions.h index 2c85385eb..f0aef6194 100644 --- a/include/dxc/Support/HLSLOptions.h +++ b/include/dxc/Support/HLSLOptions.h @@ -133,6 +133,7 @@ public: std::vector PreciseOutputs; // OPT_precise_output llvm::StringRef DefaultLinkage; // OPT_default_linkage llvm::StringRef ImportBindingTable; // OPT_import_binding_table + llvm::StringRef BindingTableDefine; // OPT_binding_table_define unsigned DefaultTextCodePage = DXC_CP_UTF8; // OPT_encoding bool AllResourcesBound = false; // OPT_all_resources_bound diff --git a/include/dxc/Support/HLSLOptions.td b/include/dxc/Support/HLSLOptions.td index fca886f41..c450d1d17 100644 --- a/include/dxc/Support/HLSLOptions.td +++ b/include/dxc/Support/HLSLOptions.td @@ -130,6 +130,8 @@ def ferror_limit_EQ : Joined<["-"], "ferror-limit=">, Group, Fla def import_binding_table : JoinedOrSeparate<["-", "/"], "import-binding-table">, Group, Flags<[CoreOption, DriverOption, HelpHidden]>, HelpText<"Import a binding table to specify resource bindings.">; +def binding_table_define : Separate<["-", "/"], "binding-table-define">, Group, Flags<[CoreOption, DriverOption, HelpHidden]>, + HelpText<"Import a binding table from a define to specify resource bindings.">; def funsafe_math_optimizations : Flag<["-"], "funsafe-math-optimizations">, Group; diff --git a/lib/DxcBindingTable/DxcBindingTable.cpp b/lib/DxcBindingTable/DxcBindingTable.cpp index 8ddbea12b..191a09f3c 100644 --- a/lib/DxcBindingTable/DxcBindingTable.cpp +++ b/lib/DxcBindingTable/DxcBindingTable.cpp @@ -61,7 +61,7 @@ bool hlsl::ParseBindingTable(llvm::StringRef fileName, llvm::StringRef content, int line = 1; int col = 1; llvm::raw_ostream &errors; - bool WasNewline = false; + bool WasEndOfLine = false; struct Location { int line = 0; @@ -74,6 +74,9 @@ bool hlsl::ParseBindingTable(llvm::StringRef fileName, llvm::StringRef content, inline static bool IsNewline(char c) { return c == '\r' || c == '\n'; } + inline static bool IsEndOfLine(char c) { + return IsNewline(c) || c == ';' || c == '\0'; + } inline static bool IsWhitespace(char c) { return c == ' ' || c == '\t'; } @@ -86,7 +89,7 @@ bool hlsl::ParseBindingTable(llvm::StringRef fileName, llvm::StringRef content, EatWhiteSpaceAndNewlines(); } inline bool WasJustEndOfLine() const { - return WasNewline; + return WasEndOfLine; } inline void EatWhitespace() { @@ -156,8 +159,8 @@ bool hlsl::ParseBindingTable(llvm::StringRef fileName, llvm::StringRef content, } while (!ReachedEnd()) { - if (IsNewline(Peek()) || (!hasQuote && IsDelimiter(Peek()))) { - if (hasQuote) + if (IsEndOfLine(Peek()) || (!hasQuote && IsDelimiter(Peek()))) { + if (hasQuote && IsNewline(Peek())) return Error("Unexpected newline inside quotation."); // Trim the white space at the end of the string if (str) { @@ -173,7 +176,7 @@ bool hlsl::ParseBindingTable(llvm::StringRef fileName, llvm::StringRef content, if (!hasQuote) return Error("'\"' not allowed in non-quoted cell."); EatWhitespace(); - if (!IsDelimiter(Peek()) && !IsNewline(Peek())) { + if (!IsDelimiter(Peek()) && !IsEndOfLine(Peek())) { return Error("Unexpected character after quote."); } break; @@ -188,8 +191,8 @@ bool hlsl::ParseBindingTable(llvm::StringRef fileName, llvm::StringRef content, // Handle delimiter { // If this delimiter is not a newline, set our newline flag to false. - if (!IsNewline(Peek())) { - WasNewline = false; + if (!IsEndOfLine(Peek())) { + WasEndOfLine = false; Advance(); // Eat white spaces so we can detect the next newline if this @@ -197,8 +200,9 @@ bool hlsl::ParseBindingTable(llvm::StringRef fileName, llvm::StringRef content, EatWhitespace(); } - if (IsNewline(Peek())) { - WasNewline = true; + if (IsEndOfLine(Peek())) { + Advance(); // Skip this character, which could be ';' + WasEndOfLine = true; EatWhiteSpaceAndNewlines(); } } diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index 64854dd4d..1d3d2a21f 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -538,6 +538,7 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, opts.AssemblyCode = Args.getLastArgValue(OPT_Fc); opts.DebugFile = Args.getLastArgValue(OPT_Fd); opts.ImportBindingTable = Args.getLastArgValue(OPT_import_binding_table); + opts.BindingTableDefine = Args.getLastArgValue(OPT_binding_table_define); opts.ExtractPrivateFile = Args.getLastArgValue(OPT_getprivate); opts.Enable16BitTypes = Args.hasFlag(OPT_enable_16bit_types, OPT_INVALID, false); opts.OutputObject = Args.getLastArgValue(OPT_Fo); diff --git a/tools/clang/include/clang/Frontend/CodeGenOptions.h b/tools/clang/include/clang/Frontend/CodeGenOptions.h index 1523f7c19..e8d79d288 100644 --- a/tools/clang/include/clang/Frontend/CodeGenOptions.h +++ b/tools/clang/include/clang/Frontend/CodeGenOptions.h @@ -246,6 +246,12 @@ public: bool HLSLEnablePayloadAccessQualifiers = false; /// Binding table for HLSL resources hlsl::DxcBindingTable HLSLBindingTable; + /// Binding table #define + struct BindingTableParserType { + virtual ~BindingTableParserType() {}; + virtual bool Parse(llvm::raw_ostream &os, hlsl::DxcBindingTable *outBindingTable) = 0; + }; + std::shared_ptr BindingTableParser; // HLSL Change Ends // SPIRV Change Starts diff --git a/tools/clang/lib/CodeGen/ModuleBuilder.cpp b/tools/clang/lib/CodeGen/ModuleBuilder.cpp index d9ab6f682..c50bed1cf 100644 --- a/tools/clang/lib/CodeGen/ModuleBuilder.cpp +++ b/tools/clang/lib/CodeGen/ModuleBuilder.cpp @@ -216,8 +216,25 @@ namespace { // HLSL Change Begins - // Add resource binding overrides to the metadata. - hlsl::WriteBindingTableToMetadata(*M, CodeGenOpts.HLSLBindingTable); + if (CodeGenOpts.BindingTableParser) { + hlsl::DxcBindingTable bindingTable; + std::string errors; + llvm::raw_string_ostream os(errors); + + if (!CodeGenOpts.BindingTableParser->Parse(os, &bindingTable)) { + os.flush(); + unsigned DiagID = Diags.getCustomDiagID( + DiagnosticsEngine::Error, "%0"); + Diags.Report(DiagID) << errors; + } + else { + hlsl::WriteBindingTableToMetadata(*M, bindingTable); + } + } + else { + // Add resource binding overrides to the metadata. + hlsl::WriteBindingTableToMetadata(*M, CodeGenOpts.HLSLBindingTable); + } // Error may happen in Builder->Release for HLSL if (CodeGenOpts.HLSLEmbedSourcesInModule) { diff --git a/tools/clang/test/HLSLFileCheck/dxil/debug/binding-table.hlsl b/tools/clang/test/HLSLFileCheck/dxil/debug/binding-table.hlsl new file mode 100644 index 000000000..b11a9429c --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/dxil/debug/binding-table.hlsl @@ -0,0 +1,125 @@ +// RUN: %dxc -WX -Od -E main -T ps_6_0 %s | FileCheck %s -check-prefixes=ERR +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define NORMAL_TABLE %s | FileCheck %s +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define REORDER %s | FileCheck %s +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define INLINE -DINLINE="resourcename,binding,space;cb,b10,0x1e;resource,b42,999;samp0,s1,0x02;resource,t1,2;uav_0,u0,0;" %s | FileCheck %s +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define EXTRA_CELL %s | FileCheck %s -check-prefixes=EXTRA_CELL +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define MISSING_CELL %s | FileCheck %s -check-prefixes=MISSING_CELL +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define MISSING_COLUMN %s | FileCheck %s -check-prefixes=MISSING_COLUMN +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define EMPTY %s | FileCheck %s -check-prefixes=EMPTY +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define INVALID_BINDING %s | FileCheck %s -check-prefixes=INVALID_BINDING +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define INVALID_BINDING_2 %s | FileCheck %s -check-prefixes=INVALID_BINDING_2 +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define INVALID_BINDING_3 %s | FileCheck %s -check-prefixes=INVALID_BINDING_3 +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define OOB %s | FileCheck %s -check-prefixes=OOB +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define OOB_2 %s | FileCheck %s -check-prefixes=OOB_2 +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define OOB_3 %s | FileCheck %s -check-prefixes=OOB_3 +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define EMPTY_CELL %s | FileCheck %s -check-prefixes=EMPTY_CELL +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define NOT_INTEGER %s | FileCheck %s -check-prefixes=NOT_INTEGER +// RUN: %dxc -WX -Od -E main -T ps_6_0 -binding-table-define NOT_FOUND %s | FileCheck %s -check-prefixes=NOT_FOUND + +// First check that the default compile fails +// ERR: Root Signature in DXIL container is not compatible with shader + +// CHECK: @main +// NOT_FOUND: Binding table define'NOT_FOUND' not found. + +#define NORMAL_TABLE \ + "resourcename, binding, space;" \ + "cb, b10, 0x1e;" \ + "resource, b42, 999; " \ + "samp0, s1, 0x02;" \ + "resource, t1, 2; " \ + "uav_0, u0, 0; " + +#define REORDER \ + " spacE, binding, ResourceName; " \ + " 0x1e, b10, cb; " \ + " 999, b42, resource; " \ + " 0x02, s1, samp0; " \ + " 2, t1, resource; " \ + " 0, u0, uav_0; " + +// EXTRA_CELL: Unexpected cell at the end of row. There should only be 3 +#define EXTRA_CELL \ + " spacE, binding, ResourceName; " \ + " 0x1e, b10, cb, extra; " \ + " 999, b42, resource; " \ + " 0x02, s1, samp0; " \ + " 2, t1, resource; " \ + " 0, u0, uav_0; " + +// MISSING_CELL: Row ended after just 2 columns. Expected 3. +#define MISSING_CELL \ + "spacE, binding, ResourceName; " \ + "0x1e, b10; " \ + "999, b42, resource; " \ + "0x02, s1, samp0; " \ + "2, t1, resource; " \ + "0, u0, uav_0; " + +// MISSING_COLUMN: Input format is csv with headings: ResourceName, Binding, Space. +#define MISSING_COLUMN \ + "resourcename, space;" \ + "cb, 0x1e; " \ + "resource, 999; " \ + "samp0, 0x02; " \ + "resource, 2; " \ + "uav_0, 0; " + +// EMPTY: Unexpected EOF when parsing cell. +#define EMPTY "" + +// INVALID_BINDING: Invalid resource class +#define INVALID_BINDING \ + "resourcename, binding, space;" \ + "cb, 10, 0x1e;" + +// INVALID_BINDING_2: Invalid resource class +#define INVALID_BINDING_2 \ + "resourcename, binding, space;" \ + "cb, e10, 0x1e;" + +// INVALID_BINDING_3: 'b' is not a valid resource binding. +#define INVALID_BINDING_3 \ + "resourcename, binding, space;" \ + "cb, b, 0x1e;" + +// OOB: '99999999999999999999999999999999999999999' is out of range of an 32-bit unsigned integer. +#define OOB \ + "resourcename, binding, space;" \ + "cb, b99999999999999999999999999999999999999999, 0x1e;" + +// OOB_2: '99999999999999999999999999999999999' is out of range of an 32-bit unsigned integer. +#define OOB_2 \ + "resourcename, binding, space;" \ + "cb, b10, 99999999999999999999999999999999999" + +// OOB_3: '0xffffffffffffffffffffffffffffffffffffffffffffffffffff' is out of range of an 32-bit unsigned integer. +#define OOB_3 \ + "resourcename, binding, space;" \ + "cb, b10, 0xffffffffffffffffffffffffffffffffffffffffffffffffffff" + +// NOT_INTEGER: 'abcd' is not a valid 32-bit unsigned integer. +#define NOT_INTEGER \ + "resourcename, binding, space;" \ + "cb, b10, abcd" + +// EMPTY_CELL: Resource binding cannot be empty. +#define EMPTY_CELL \ + "resourcename, binding, space;" \ + "cb, , 0x1e;" + +cbuffer cb { + float a; +}; +cbuffer resource { + float b; +}; + +SamplerState samp0; +Texture2D resource; +RWTexture1D uav_0; + +[RootSignature("CBV(b10,space=30), CBV(b42,space=999), DescriptorTable(Sampler(s1,space=2)), DescriptorTable(SRV(t1,space=2)), DescriptorTable(UAV(u0,space=0))")] +float main(float2 uv : UV, uint i : I) :SV_Target { + return a + b + resource.Sample(samp0, uv).r + uav_0[i]; +} \ No newline at end of file diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 0bb5804dd..2e2f2635a 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -816,7 +816,41 @@ public: compiler.getCodeGenOpts().HLSLProfile = opts.TargetProfile; // Parse and apply - if (opts.ImportBindingTable.size()) { + if (opts.BindingTableDefine.size()) { + // Just pas the define for now because preprocessor is not available yet. + struct BindingTableParserImpl : public CodeGenOptions::BindingTableParserType { + CompilerInstance &compiler; + std::string define; + BindingTableParserImpl(CompilerInstance &compiler, StringRef define) + :compiler(compiler), define(define.str()) + {} + + bool Parse(llvm::raw_ostream &os, hlsl::DxcBindingTable *outBindingTable) override { + Preprocessor &pp = compiler.getPreprocessor(); + MacroInfo *macro = MacroExpander::FindMacroInfo(pp, define); + if (!macro) { + os << Twine("Binding table define'") + define + "' not found."; + os.flush(); + return false; + } + + std::string bindingTableStr; + // Combine tokens into single string + MacroExpander expander(pp, MacroExpander::STRIP_QUOTES); + if (!expander.ExpandMacro(macro, &bindingTableStr)) { + os << Twine("Binding table define'") + define + "' failed to expand."; + os.flush(); + return false; + } + return hlsl::ParseBindingTable( + define, StringRef(bindingTableStr), + os, outBindingTable); + } + }; + + compiler.getCodeGenOpts().BindingTableParser.reset(new BindingTableParserImpl(compiler, opts.BindingTableDefine)); + } + else if (opts.ImportBindingTable.size()) { hlsl::options::StringRefUtf16 wstrRef(opts.ImportBindingTable); CComPtr pBlob; std::string error;