spirv-opt: A pass repairs interfaces of OpEntryPoint instructions. #4275
Conversation
|
|
thatname
changed the title
There was a problem hiding this comment.
I can see that this will be useful. Thanks for adding it. I do have a few suggested changes. We have a number of utility function to try to reduce the boiler plate code that is needed. I would suggest using those unless there is a good reason not to. I may have missed something.
You should also add tests for the pass. You can look at
SPIRV-Tools/test/opt/wrap_opkill_test.cpp
Line 26 in 010cd28
|
microsoft/DirectXShaderCompiler#3789 handles the DXC issue. If the PR works correct, we do not need this change. |
…ns (#3789) As mentioned in #3759 and KhronosGroup/SPIRV-Tools#4275, DXC does not generate correct operands of `OpEntryPoint` when the shader has multiple entry functions. The main reason is that the existing SPIR-V backend does not separate interfaces between entry functions. This commit lets `DeclResultIdMapper` keep the information of the entry point of each stage variable in `StageVar` and lets `SpirvEmitter` correctly emit interfaces for each `OpEntryPoint`.
|
I submitted the change. I think we can close this issue. @thatname please let us know if it is ok or not. |
|
Let me close it now. If you have any further questions or issues, please let us know. |
|
Many tests failed. It might be because you need to rebase, and you might need to run clang-format on the new header file. Sorry for the extra work. |
|
@s-perron Thanks, I've formatted the files, now CI seems OK, still need to rebase? |
|
The format is good, but the tests still fail. We run the tests with the latest from all of our dependencies. If you update spirv-headers you will see the failure locally. The problem as already been fixed, but you need to rebase you change to pick up that fix. FYI: A maintainer has to trigger the kokoro tests for security reasons. |
…structions by listing the global variables exactly referenced.
Following the style of the code base, changed methods to begin with capital letters.
|
Ignoring the smoke test failure. It looked like a network issue, and this change will not affect it. |
…s. (KhronosGroup#4275) The new pass will removed interface variable on the OpEntryPoint instruction when they are not statically referenced in the call tree of the entry point. It can be enabled on the command line using the options `remove-unused-interface-variables`.
Hi !
Working with SPIR-V with multiple OpEntryPoint instructions is painful.
Some tools often generate incorrect or suboptimal interfaces for OpEntryPoint instructions.
If the interface of an entry point instruction lists an unused global variable that used by another entry point, currently DCE passs can not remove it from the list, because it can only remove dead variables.
If an used global variable is not listed, there is no tool to fix currently.
This PR adds spvtools::opt::InterfaceRepairPass which is an optmizer pass fixs the problem. It iterates the entry points, traverses the functions called, remembers the used variables, and rewrites the interfaces. Just add "--interface-repair" in command line to invoke it.