From 5648e43f189bfb45c3ed0b7f7989949a6240bcbb Mon Sep 17 00:00:00 2001 From: Ehsan Date: Wed, 1 May 2019 18:28:14 -0400 Subject: [PATCH] [spirv] Command line option for providing $Globals binding. (#2156) --- docs/SPIR-V.rst | 44 +++++++++++++++++++ include/dxc/Support/HLSLOptions.td | 2 + include/dxc/Support/SPIRVOptions.h | 1 + lib/DxcSupport/HLSLOptions.cpp | 2 + tools/clang/lib/SPIRV/DeclResultIdMapper.cpp | 42 ++++++++++++++++-- tools/clang/lib/SPIRV/DeclResultIdMapper.h | 7 ++- .../CodeGenSPIRV/vk.binding.cl.globals.hlsl | 12 +++++ .../vk.binding.cl.register-and-globals.hlsl | 18 ++++++++ .../unittests/SPIRV/CodeGenSpirvTest.cpp | 10 +++++ 9 files changed, 132 insertions(+), 6 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/vk.binding.cl.globals.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/vk.binding.cl.register-and-globals.hlsl diff --git a/docs/SPIR-V.rst b/docs/SPIR-V.rst index c6c45b751..d8fb4d087 100644 --- a/docs/SPIR-V.rst +++ b/docs/SPIR-V.rst @@ -1502,7 +1502,48 @@ If we compile with ``-fvk-t-shift 10 0 -fvk-t-shift 20 1``: - ``sampler1`` will take binding 1 in set #0, since that's the next available binding number in set #0. +HLSL global variables and Vulkan binding +---------------------------------------- +As mentioned above, all global externally-visible non-resource-type stand-alone +variables will be collected into a cbuffer named ``$Globals``. By default, +the ``$Globals`` cbuffer is placed in descriptor set #0, and the binding number +would be the next available binding number in that set. Meaning, the binding number +depends on where the very first global variable is in the code. + +Example 1: + .. code:: hlsl + + float4 someColors; + // $Globals cbuffer placed at DescriptorSet #0, Binding #0 + Texture2D texture1; + // texture1 placed at DescriptorSet #0, Binding #1 + +Example 2: + +.. code:: hlsl + + Texture2D texture1; + // texture1 placed at DescriptorSet #0, Binding #0 + float4 someColors; + // $Globals cbuffer placed at DescriptorSet #0, Binding #1 + +In order provide more control over the descriptor set and binding number of the +``$Globals`` cbuffer, you can use the ``-fvk-bind-globals B S`` command line +option, which will place this cbuffer at descriptor set ``S``, and binding number ``B``. + +Example 3: (compiled with ``-fvk-bind-globals 2 1``) + +.. code:: hlsl + + Texture2D texture1; + // texture1 placed at DescriptorSet #0, Binding #0 + float4 someColors; + // $Globals cbuffer placed at DescriptorSet #1, Binding #2 + +Note that if the developer chooses to use this command line option, it is their +responsibility to provide proper numbers and avoid binding overlaps. + HLSL Expressions ================ @@ -3202,6 +3243,9 @@ codegen for Vulkan: It requires all source code resources have ``:register()`` attribute and all registers have corresponding Vulkan descriptors specified using this option. +- ``-fvk-bind-globals N M``: Places the ``$Globals`` cbuffer at + descriptor set #M and binding #N. See `HLSL global variables and Vulkan binding`_ + for explanation and examples. - ``-fvk-use-gl-layout``: Uses strict OpenGL ``std140``/``std430`` layout rules for resources. - ``-fvk-use-dx-layout``: Uses DirectX layout rules for resources. diff --git a/include/dxc/Support/HLSLOptions.td b/include/dxc/Support/HLSLOptions.td index 18d056feb..044561f56 100644 --- a/include/dxc/Support/HLSLOptions.td +++ b/include/dxc/Support/HLSLOptions.td @@ -256,6 +256,8 @@ def fvk_s_shift : MultiArg<["-"], "fvk-s-shift", 2>, MetaVarName<" ; def fvk_u_shift : MultiArg<["-"], "fvk-u-shift", 2>, MetaVarName<" ">, Group, Flags<[CoreOption, DriverOption]>, HelpText<"Specify Vulkan binding number shift for u-type register">; +def fvk_bind_globals : MultiArg<["-"], "fvk-bind-globals", 2>, MetaVarName<" ">, Group, Flags<[CoreOption, DriverOption]>, + HelpText<"Specify Vulkan binding number and set number for the $Globals cbuffer">; def fvk_bind_register : MultiArg<["-"], "fvk-bind-register", 4>, MetaVarName<" ">, Group, Flags<[CoreOption, DriverOption]>, HelpText<"Specify Vulkan descriptor set and binding for a specific register">; def vkbr : MultiArg<["-"], "vkbr", 4>, Flags<[CoreOption, DriverOption]>, Alias; diff --git a/include/dxc/Support/SPIRVOptions.h b/include/dxc/Support/SPIRVOptions.h index 4a4a72720..85aa93821 100644 --- a/include/dxc/Support/SPIRVOptions.h +++ b/include/dxc/Support/SPIRVOptions.h @@ -65,6 +65,7 @@ struct SpirvCodeGenOptions { llvm::SmallVector allowedExtensions; llvm::SmallVector optConfig; std::vector bindRegister; + std::vector bindGlobals; // String representation of all command line options. std::string clOptions; diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index 671cf8860..10c16f4ef 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -682,6 +682,7 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, return 1; opts.SpirvOptions.bindRegister = Args.getAllArgValues(OPT_fvk_bind_register); + opts.SpirvOptions.bindGlobals = Args.getAllArgValues(OPT_fvk_bind_globals); opts.SpirvOptions.stageIoOrder = Args.getLastArgValue(OPT_fvk_stage_io_order_EQ, "decl"); if (opts.SpirvOptions.stageIoOrder != "alpha" && opts.SpirvOptions.stageIoOrder != "decl") { errors << "unknown Vulkan stage I/O location assignment order: " @@ -756,6 +757,7 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, !Args.getLastArgValue(OPT_fspv_target_env_EQ).empty() || !Args.getLastArgValue(OPT_Oconfig).empty() || !Args.getLastArgValue(OPT_fvk_bind_register).empty() || + !Args.getLastArgValue(OPT_fvk_bind_globals).empty() || !Args.getLastArgValue(OPT_fvk_b_shift).empty() || !Args.getLastArgValue(OPT_fvk_t_shift).empty() || !Args.getLastArgValue(OPT_fvk_s_shift).empty() || diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp index 57095c895..92eed021e 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp @@ -862,7 +862,8 @@ void DeclResultIdMapper::createGlobalsCBuffer(const VarDecl *var) { "$Globals"); resourceVars.emplace_back(globals, SourceLocation(), nullptr, nullptr, - nullptr); + nullptr, /*isCounterVar*/ false, + /*isGlobalsCBuffer*/ true); uint32_t index = 0; for (const auto *decl : collectDeclsInDeclContext(context)) @@ -1371,6 +1372,25 @@ bool DeclResultIdMapper::decorateResourceBindings() { // - m2 // - m3, m4, mX * c2 + const bool bindGlobals = !spirvOptions.bindGlobals.empty(); + int32_t globalsBindNo = -1, globalsSetNo = -1; + if (bindGlobals) { + assert(spirvOptions.bindGlobals.size() == 2); + if (StringRef(spirvOptions.bindGlobals[0]) + .getAsInteger(10, globalsBindNo) || + globalsBindNo < 0) { + emitError("invalid -fvk-bind-globals binding number: %0", {}) + << spirvOptions.bindGlobals[0]; + return false; + } + if (StringRef(spirvOptions.bindGlobals[1]).getAsInteger(10, globalsSetNo) || + globalsSetNo < 0) { + emitError("invalid -fvk-bind-globals set number: %0", {}) + << spirvOptions.bindGlobals[1]; + return false; + } + } + // Special handling of -fvk-bind-register, which requires // * All resources are annoated with :register() in the source code // * -fvk-bind-register is specified for every resource @@ -1398,6 +1418,9 @@ bool DeclResultIdMapper::decorateResourceBindings() { } spvBuilder.decorateDSetBinding(var.getSpirvInstr(), setNo, bindNo); } + } else if (bindGlobals && var.isGlobalsBuffer()) { + spvBuilder.decorateDSetBinding(var.getSpirvInstr(), globalsSetNo, + globalsBindNo); } else { emitError( "-fvk-bind-register requires register annotations on all resources", @@ -1498,9 +1521,20 @@ bool DeclResultIdMapper::decorateResourceBindings() { spvBuilder.decorateDSetBinding(var.getSpirvInstr(), set, bindingSet.useNextBinding(set)); } else if (!reg) { - // Process m3 - spvBuilder.decorateDSetBinding(var.getSpirvInstr(), 0, - bindingSet.useNextBinding(0)); + // Process m3 (no 'vk::binding' and no ':register' assignment) + + // There is a special case for the $Globals cbuffer. The $Globals buffer + // doesn't have either 'vk::binding' or ':register', but the user may + // ask for a specific binding for it via command line options. + if (bindGlobals && var.isGlobalsBuffer()) { + spvBuilder.decorateDSetBinding(var.getSpirvInstr(), globalsSetNo, + globalsBindNo); + } + // The normal case + else { + spvBuilder.decorateDSetBinding(var.getSpirvInstr(), 0, + bindingSet.useNextBinding(0)); + } } } } diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.h b/tools/clang/lib/SPIRV/DeclResultIdMapper.h index 202a5f51a..cbaf7282b 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.h +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.h @@ -114,15 +114,17 @@ class ResourceVar { public: ResourceVar(SpirvVariable *var, SourceLocation loc, const hlsl::RegisterAssignment *r, const VKBindingAttr *b, - const VKCounterBindingAttr *cb, bool counter = false) + const VKCounterBindingAttr *cb, bool counter = false, + bool globalsBuffer = false) : variable(var), srcLoc(loc), reg(r), binding(b), counterBinding(cb), - isCounterVar(counter) {} + isCounterVar(counter), isGlobalsCBuffer(globalsBuffer) {} SpirvVariable *getSpirvInstr() const { return variable; } SourceLocation getSourceLocation() const { return srcLoc; } const hlsl::RegisterAssignment *getRegister() const { return reg; } const VKBindingAttr *getBinding() const { return binding; } bool isCounter() const { return isCounterVar; } + bool isGlobalsBuffer() const { return isGlobalsCBuffer; } const VKCounterBindingAttr *getCounterBinding() const { return counterBinding; } @@ -134,6 +136,7 @@ private: const VKBindingAttr *binding; ///< Vulkan binding assignment const VKCounterBindingAttr *counterBinding; ///< Vulkan counter binding bool isCounterVar; ///< Couter variable or not + bool isGlobalsCBuffer; ///< $Globals cbuffer or not }; /// A (instruction-pointer, is-alias-or-not) pair for counter variables diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.cl.globals.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.globals.hlsl new file mode 100644 index 000000000..dde563149 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.globals.hlsl @@ -0,0 +1,12 @@ +// Run: %dxc -T ps_6_0 -E main -fvk-bind-globals 1 2 + +// CHECK: OpDecorate %_Globals DescriptorSet 2 +// CHECK: OpDecorate %_Globals Binding 1 + +int globalInteger; +float4 globalFloat4; + +float4 main() : SV_Target { + return (globalInteger + globalFloat4.z).xxxx; +} + diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.cl.register-and-globals.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.register-and-globals.hlsl new file mode 100644 index 000000000..fbaecab13 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.register-and-globals.hlsl @@ -0,0 +1,18 @@ +// Run: %dxc -T ps_6_0 -E main -fvk-bind-register t5 0 1 2 -vkbr s3 1 3 4 -fvk-bind-globals 7 8 + +// CHECK: OpDecorate %MyTexture DescriptorSet 2 +// CHECK: OpDecorate %MyTexture Binding 1 +Texture2D MyTexture : register(t5); +// CHECK: OpDecorate %MySampler DescriptorSet 4 +// CHECK: OpDecorate %MySampler Binding 3 +SamplerState MySampler : register(s3, space1); + +// CHECK: OpDecorate %_Globals DescriptorSet 8 +// CHECK: OpDecorate %_Globals Binding 7 +int globalInteger; +float4 globalFloat4; + +float4 main() : SV_Target { + return MyTexture.Sample(MySampler, float2(0.1, 0.2)); +} + diff --git a/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp b/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp index a462377de..264871899 100644 --- a/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp +++ b/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp @@ -1523,6 +1523,16 @@ TEST_F(FileTest, VulkanRegisterBindingShiftAllSets) { TEST_F(FileTest, VulkanRegisterBinding1to1Mapping) { runFileTest("vk.binding.cl.register.hlsl"); } +TEST_F(FileTest, VulkanGlobalsBinding) { + // Binding the $Globals buffer to a specific set/binding via command line + // option. + runFileTest("vk.binding.cl.globals.hlsl"); +} +TEST_F(FileTest, VulkanGlobalsBindingRegisterBinding) { + // Using command line option for specifying both the 1-1 register mapping as + // well as $Globals binding. + runFileTest("vk.binding.cl.register-and-globals.hlsl"); +} TEST_F(FileTest, VulkanRegisterBinding1to1MappingInvalidSpaceNo) { runFileTest("vk.binding.cl.register.invalid-space.hlsl", Expect::Failure); }