Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spirv-opt generates code that spirv-val rejects: ID ... has not been defined #2455

Closed
afd opened this issue Mar 15, 2019 · 3 comments
Closed
Assignees

Comments

@afd
Copy link
Contributor

afd commented Mar 15, 2019

Files to reproduce problem

glsl_reduced.frag is a minimized GLSL shader. When compiled to SPIR-V with glslangValidator we get glsl_reduced.spv. This has been further reduced using spirv-reduce to spiv_reduced.spv.

Doing:

spirv-opt spirv_reduced.spv --scalar-replacement=100 --vector-dce --eliminate-local-single-block --redundancy-elimination --eliminate-dead-branches --merge-return --convert-local-access-chains --eliminate-dead-code-aggressive -o temp.spv

spirv-val temp.spv

should trigger the problem:

error: line 96: ID 95[%95] has not been defined
OpBranchConditional %95 %62 %64

I did not try to reduce the spirv-opt flags, in case (as with #2450) there may be an issue both with the optimizer generating invalid code and the validator not picking this up soon enough.

Found using GraphicsFuzz

@alan-baker alan-baker self-assigned this Mar 18, 2019
@alan-baker
Copy link
Contributor

Similar to #2453. In this case, merge return adds the OpPhi, but in the wrong block. That leads to the bad definition.

@alan-baker
Copy link
Contributor

Issue is specifically updating users when adding OpPhi instructions.

@alan-baker
Copy link
Contributor

The instruction builder that adds phis doesn't keep the instruction to block mapping updated and that leads to dominator information being incorrect.

alan-baker added a commit to alan-baker/SPIRV-Tools that referenced this issue Mar 19, 2019
Fixes KhronosGroup#2455

* Properly maintains instruction to block mapping for newly created phi
instructions in merge return
* adds a test
s-perron pushed a commit that referenced this issue Apr 1, 2019
Fixes #2455

Properly maintains instruction to block mapping for newly created phi instructions in merge return
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants