From 4b7993c78be1b90b2b9d410813c77ca87af22097 Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Mon, 10 Jun 2024 14:13:01 -0400 Subject: [PATCH] Add option to set the max id (#6654) Vulkan implementation can have different limits on the maximum value used as an id in a SPIR-V binary. SPIRV-Tools generall assumes this limit is 0x3FFFFF because all implementations must support at least that value for an id. Since many implementations can support larger values, the tools allows an option that will set a different limit. This commit add an option to DXC to do the same. Fixes #6636 --- include/dxc/Support/HLSLOptions.td | 2 ++ include/dxc/Support/SPIRVOptions.h | 1 + lib/DxcSupport/HLSLOptions.cpp | 9 +++++++++ tools/clang/lib/SPIRV/SpirvEmitter.cpp | 6 ++++++ tools/clang/test/CodeGenSPIRV/max_id.hlsl | 23 +++++++++++++++++++++++ 5 files changed, 41 insertions(+) create mode 100644 tools/clang/test/CodeGenSPIRV/max_id.hlsl diff --git a/include/dxc/Support/HLSLOptions.td b/include/dxc/Support/HLSLOptions.td index d15a8c14b9..cd4eb830d0 100644 --- a/include/dxc/Support/HLSLOptions.td +++ b/include/dxc/Support/HLSLOptions.td @@ -412,6 +412,8 @@ def fspv_preserve_interface : Flag<["-"], "fspv-preserve-interface">, Group; def fvk_allow_rwstructuredbuffer_arrays: Flag<["-"], "fvk-allow-rwstructuredbuffer-arrays">, Group, Flags<[CoreOption, DriverOption, HelpHidden]>, HelpText<"Allow arrays of RWStructuredBuffers, AppendStructuredBuffers, and ConsumeStructuredBuffers. This is in development, and the option will be removed when the feature is complete.">; +def fspv_max_id : MultiArg<["-"], "fspv-max-id", 1>, MetaVarName<" ">, Group, Flags<[CoreOption, DriverOption]>, + HelpText<"Set the maximum value for an id in the SPIR-V binary. Default is 0x3FFFFF, which is the largest value all drivers must support.">; // SPIRV Change Ends ////////////////////////////////////////////////////////////////////////////// diff --git a/include/dxc/Support/SPIRVOptions.h b/include/dxc/Support/SPIRVOptions.h index 0cf521b56b..950c764367 100644 --- a/include/dxc/Support/SPIRVOptions.h +++ b/include/dxc/Support/SPIRVOptions.h @@ -83,6 +83,7 @@ struct SpirvCodeGenOptions { SpirvLayoutRule ampPayloadLayoutRule; llvm::StringRef stageIoOrder; llvm::StringRef targetEnv; + uint32_t maxId; llvm::SmallVector bShift; llvm::SmallVector sShift; llvm::SmallVector tShift; diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index e61290030b..94103077f2 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -1179,6 +1179,15 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, opts.SpirvOptions.targetEnv = Args.getLastArgValue(OPT_fspv_target_env_EQ, "vulkan1.0"); + llvm::APInt maxId; + + // 0X3FFFFF is the default value for -fspv-max-id because it is the largest + // value that is guaranteed to be allowed in all Vulkan implementations. + if (Args.getLastArgValue(OPT_fspv_max_id, "3FFFFF").getAsInteger(16, maxId)) { + errors << "-fspv-max-id must be an integer in hexadecimal format"; + } + opts.SpirvOptions.maxId = maxId.getLimitedValue(0xFFFFFFFF); + // Handle -Oconfig= option. uint32_t numOconfigs = 0; for (const Arg *A : Args.filtered(OPT_Oconfig)) { diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 4ea2f8d34d..6d0ac6f05e 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -14415,6 +14415,8 @@ bool SpirvEmitter::spirvToolsValidate(std::vector *mod, } else { options.SetRelaxBlockLayout(true); } + options.SetUniversalLimit(spv_validator_limit_max_id_bound, + spirvOptions.maxId); return tools.Validate(mod->data(), mod->size(), options); } @@ -14487,6 +14489,7 @@ bool SpirvEmitter::spirvToolsTrimCapabilities(std::vector *mod, spvtools::OptimizerOptions options; options.set_run_validator(false); options.set_preserve_bindings(spirvOptions.preserveBindings); + options.set_max_id_bound(spirvOptions.maxId); optimizer.RegisterPass(spvtools::CreateTrimCapabilitiesPass()); @@ -14509,6 +14512,7 @@ bool SpirvEmitter::spirvToolsOptimize(std::vector *mod, spvtools::OptimizerOptions options; options.set_run_validator(false); options.set_preserve_bindings(spirvOptions.preserveBindings); + options.set_max_id_bound(spirvOptions.maxId); if (spirvOptions.optConfig.empty()) { // Add performance passes. @@ -14550,6 +14554,8 @@ bool SpirvEmitter::spirvToolsLegalize(std::vector *mod, spvtools::OptimizerOptions options; options.set_run_validator(false); options.set_preserve_bindings(spirvOptions.preserveBindings); + options.set_max_id_bound(spirvOptions.maxId); + // Add interface variable SROA if the signature packing is enabled. if (spirvOptions.signaturePacking) { optimizer.RegisterPass( diff --git a/tools/clang/test/CodeGenSPIRV/max_id.hlsl b/tools/clang/test/CodeGenSPIRV/max_id.hlsl new file mode 100644 index 0000000000..d57267b399 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/max_id.hlsl @@ -0,0 +1,23 @@ +// RUN: not %dxc -T cs_6_0 -E main %s -spirv -fspv-max-id 30 2>&1 | FileCheck %s --check-prefix=CHECK-30 +// RUN: %dxc -T cs_6_0 -E main %s -spirv -fspv-max-id 400 2>&1 | FileCheck %s --check-prefix=CHECK-400 + +// With a lower limit, there will be an ID overflow in the optimizer. Note that +// the error message can vary depending on where the optimizer fails. This test +// is low enough to fail as early as possible leading to a consistent error +// message. +// CHECK-30: fatal error: failed to optimize SPIR-V: ID overflow. Try running compact-ids. + +// With a larger limit, the test case can compile successfully. +// CHECK-400: Bound: 204 + + +RWStructuredBuffer data; + +[numthreads(1,1,1)] +void main(uint3 id : SV_DispatchThreadID) +{ + [[unroll]] + for( int i = 0; i < 64; i++ ) { + data[i] = i; + } +} \ No newline at end of file