From 50c0199c17b0b7530cbc0bdddadc3697eaece695 Mon Sep 17 00:00:00 2001 From: JiaoluAMD Date: Sat, 14 May 2022 02:39:08 +0800 Subject: [PATCH] [spirv] Add FixFuncCallArguments options (#4439) 1) promotion of spirv-opt `FixFuncCallArgumentsPass` 2) add tests for the FixFuncCallArguments for the compute shader and export linkage shaders --- include/dxc/Support/HLSLOptions.td | 2 + include/dxc/Support/SPIRVOptions.h | 1 + lib/DxcSupport/HLSLOptions.cpp | 3 ++ tools/clang/lib/SPIRV/SpirvEmitter.cpp | 3 ++ .../CodeGenSPIRV/fn.fixfuncall-compute.hlsl | 34 +++++++++++++++++ .../CodeGenSPIRV/fn.fixfuncall-linkage.hlsl | 38 +++++++++++++++++++ .../unittests/SPIRV/CodeGenSpirvTest.cpp | 6 +++ 7 files changed, 87 insertions(+) create mode 100644 tools/clang/test/CodeGenSPIRV/fn.fixfuncall-compute.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/fn.fixfuncall-linkage.hlsl diff --git a/include/dxc/Support/HLSLOptions.td b/include/dxc/Support/HLSLOptions.td index bfe8db0d6..576f585e9 100644 --- a/include/dxc/Support/HLSLOptions.td +++ b/include/dxc/Support/HLSLOptions.td @@ -357,6 +357,8 @@ def fspv_flatten_resource_arrays: Flag<["-"], "fspv-flatten-resource-arrays">, G HelpText<"Flatten arrays of resources so each array element takes one binding number">; def fspv_reduce_load_size: Flag<["-"], "fspv-reduce-load-size">, Group, Flags<[CoreOption, DriverOption]>, HelpText<"Replaces loads of composite objects to reduce memory pressure for the loads">; +def fspv_fix_func_call_arguments: Flag<["-"], "fspv-fix-func-call-arguments">, Group, Flags<[CoreOption, DriverOption, HelpHidden]>, + HelpText<"Fix function call arguments which are not memory objects">; def fspv_entrypoint_name_EQ : Joined<["-"], "fspv-entrypoint-name=">, Group, Flags<[CoreOption, DriverOption]>, HelpText<"Specify the SPIR-V entry point name. Defaults to the HLSL entry point name.">; def fvk_auto_shift_bindings: Flag<["-"], "fvk-auto-shift-bindings">, Group, Flags<[CoreOption, DriverOption]>, diff --git a/include/dxc/Support/SPIRVOptions.h b/include/dxc/Support/SPIRVOptions.h index 944af59c8..890031e3b 100644 --- a/include/dxc/Support/SPIRVOptions.h +++ b/include/dxc/Support/SPIRVOptions.h @@ -61,6 +61,7 @@ struct SpirvCodeGenOptions { bool reduceLoadSize; bool autoShiftBindings; bool supportNonzeroBaseInstance; + bool fixFuncCallArguments; /// Maximum length in words for the OpString literal containing the shader /// source for DebugSource and DebugSourceContinued. If the source code length /// is larger than this number, we will use DebugSourceContinued instructions diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index 0c0a7afec..9aaa47ee0 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -963,6 +963,8 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, Args.hasFlag(OPT_fspv_flatten_resource_arrays, OPT_INVALID, false); opts.SpirvOptions.reduceLoadSize = Args.hasFlag(OPT_fspv_reduce_load_size, OPT_INVALID, false); + opts.SpirvOptions.fixFuncCallArguments = + Args.hasFlag(OPT_fspv_fix_func_call_arguments, OPT_INVALID, false); opts.SpirvOptions.autoShiftBindings = Args.hasFlag(OPT_fvk_auto_shift_bindings, OPT_INVALID, false); if (!handleVkShiftArgs(Args, OPT_fvk_b_shift, "b", &opts.SpirvOptions.bShift, errors) || @@ -1078,6 +1080,7 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, Args.hasFlag(OPT_fspv_flatten_resource_arrays, OPT_INVALID, false) || Args.hasFlag(OPT_fspv_reduce_load_size, OPT_INVALID, false) || Args.hasFlag(OPT_fspv_reflect, OPT_INVALID, false) || + Args.hasFlag(OPT_fspv_fix_func_call_arguments, OPT_INVALID, false) || Args.hasFlag(OPT_Wno_vk_ignored_features, OPT_INVALID, false) || Args.hasFlag(OPT_Wno_vk_emulated_features, OPT_INVALID, false) || Args.hasFlag(OPT_fvk_auto_shift_bindings, OPT_INVALID, false) || diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 4eadc4200..a41f96ec3 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -13111,6 +13111,9 @@ bool SpirvEmitter::spirvToolsLegalize(std::vector *mod, optimizer.RegisterPass(spvtools::CreateReplaceInvalidOpcodePass()); optimizer.RegisterPass(spvtools::CreateCompactIdsPass()); optimizer.RegisterPass(spvtools::CreateSpreadVolatileSemanticsPass()); + if (spirvOptions.fixFuncCallArguments) { + optimizer.RegisterPass(spvtools::CreateFixFuncCallArgumentsPass()); + } return optimizer.Run(mod->data(), mod->size(), mod, options); } diff --git a/tools/clang/test/CodeGenSPIRV/fn.fixfuncall-compute.hlsl b/tools/clang/test/CodeGenSPIRV/fn.fixfuncall-compute.hlsl new file mode 100644 index 000000000..daad5393e --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/fn.fixfuncall-compute.hlsl @@ -0,0 +1,34 @@ +// RUN: %dxc -T cs_6_0 -E main -fspv-fix-func-call-arguments -O0 +RWStructuredBuffer< float4 > output : register(u1); + +[noinline] +float4 foo(inout float f0, inout int f1) +{ + return 0; +} + +// CHECK: [[s39:%\w+]] = OpVariable %_ptr_Function_int Function +// CHECK: [[s36:%\w+]] = OpVariable %_ptr_Function_float Function +// CHECK: [[s33:%\w+]] = OpAccessChain %_ptr_Uniform_float {{%\w+}} %int_0 +// CHECK: [[s34:%\w+]] = OpAccessChain %_ptr_Function_int {{%\w+}} %int_1 +// CHECK: [[s37:%\w+]] = OpLoad %float [[s33]] +// CHECK: OpStore [[s36]] [[s37]] +// CHECK: [[s40:%\w+]] = OpLoad %int [[s34]] +// CHECK: OpStore [[s39]] [[s40]] +// CHECK: {{%\w+}} = OpFunctionCall %v4float %foo [[s36]] [[s39]] +// CHECK: [[s41:%\w+]] = OpLoad %int [[s39]] +// CHECK: OpStore [[s34]] [[s41]] +// CHECK: [[s38:%\w+]] = OpLoad %float [[s36]] +// CHECK: OpStore [[s33]] [[s38]] + +struct Stru { + int x; + int y; +}; + +[numthreads(1,1,1)] +void main() +{ + Stru stru; + foo(output[0].x, stru.y); +} diff --git a/tools/clang/test/CodeGenSPIRV/fn.fixfuncall-linkage.hlsl b/tools/clang/test/CodeGenSPIRV/fn.fixfuncall-linkage.hlsl new file mode 100644 index 000000000..f2250e67d --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/fn.fixfuncall-linkage.hlsl @@ -0,0 +1,38 @@ +// RUN: %dxc -T lib_6_3 -fspv-target-env=universal1.5 -fspv-fix-func-call-arguments -O0 + +// CHECK: OpCapability Shader +// CHECK: OpCapability Linkage +RWStructuredBuffer< float4 > output : register(u1); + +// CHECK: OpDecorate %main LinkageAttributes "main" Export +// CHECK: %main = OpFunction %int None +// CHECK: [[s39:%\w+]] = OpVariable %_ptr_Function_int Function +// CHECK: [[s36:%\w+]] = OpVariable %_ptr_Function_float Function +// CHECK: [[s33:%\w+]] = OpAccessChain %_ptr_StorageBuffer_float {{%\w+}} %int_0 +// CHECK: [[s34:%\w+]] = OpAccessChain %_ptr_Function_int %stru %int_1 +// CHECK: [[s37:%\w+]] = OpLoad %float [[s33]] +// CHECK: OpStore [[s36]] [[s37]] +// CHECK: [[s40:%\w+]] = OpLoad %int [[s34]] +// CHECK: OpStore [[s39]] [[s40]] +// CHECK: {{%\w+}} = OpFunctionCall %void %func [[s36]] [[s39]] +// CHECK: [[s41:%\w+]] = OpLoad %int [[s39]] +// CHECK: OpStore [[s34]] [[s41]] +// CHECK: [[s38:%\w+]] = OpLoad %float [[s36]] +// CHECK: OpStore [[s33]] [[s38]] + +[noinline] +void func(inout float f0, inout int f1) { + +} + +struct Stru { + int x; + int y; +}; + +export int main(inout float4 color) { + output[0] = color; + Stru stru; + func(output[0].x, stru.y); + return 1; +} diff --git a/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp b/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp index 66e1ce828..5441fc024 100644 --- a/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp +++ b/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp @@ -642,6 +642,12 @@ TEST_F(FileTest, FunctionInCTBuffer) { TEST_F(FileTest, FunctionNoInline) { runFileTest("fn.noinline.hlsl"); } TEST_F(FileTest, FunctionExport) { runFileTest("fn.export.hlsl"); } + +TEST_F(FileTest, FixFunctionCall) { + runFileTest("fn.fixfuncall-compute.hlsl"); + runFileTest("fn.fixfuncall-linkage.hlsl"); +} + TEST_F(FileTest, FunctionForwardDecl) { runFileTest("fn.forward-declaration.hlsl"); }