Skip to content

Do not use PropagateLineInfoPass and RedundantLineInfoElimPass#2440

Merged
johnkslang merged 2 commits intoKhronosGroup:masterfrom
jaebaek:opline
Nov 2, 2020
Merged

Do not use PropagateLineInfoPass and RedundantLineInfoElimPass#2440
johnkslang merged 2 commits intoKhronosGroup:masterfrom
jaebaek:opline

Conversation

@jaebaek
Copy link
Copy Markdown
Contributor

@jaebaek jaebaek commented Oct 28, 2020

Since spirv-opt will remove PropagateLineInfoPass and
RedundantLineInfoElimPass, glslang should not use it. spirv-opt will
propagate the line instructions and eliminate the redundant lines by
default in IR loading/emission.

Since spirv-opt will remove PropagateLineInfoPass and
RedundantLineInfoElimPass, glslang should not use it. spirv-opt will
propagate the line instructions and eliminate the redundant lines by
default in IR loading/emission.
@jaebaek
Copy link
Copy Markdown
Contributor Author

jaebaek commented Oct 28, 2020

This PR is related to KhronosGroup/SPIRV-Tools#3951.
KhronosGroup/SPIRV-Tools#3951 will remove PropagateLineInfoPass and RedundantLineInfoElimPass. It will propagate the line instructions and eliminate the redundant lines by
default. This change will fix the shaderc smoke test failure of KhronosGroup/SPIRV-Tools#3951.

@johnkslang
Copy link
Copy Markdown
Contributor

It looks like you didn't update the test results, or if they should have been the same, something more subtle is going on.

@jaebaek
Copy link
Copy Markdown
Contributor Author

jaebaek commented Oct 28, 2020

Right. I will update this PR after submitting KhronosGroup/SPIRV-Tools#3951 first. It's a chicken and an egg issue. They both fail because of each other.

I will get back to you after submitting the spirv-opt PR. Thanks!

jaebaek added a commit to KhronosGroup/SPIRV-Tools that referenced this pull request Oct 29, 2020
Based on the OpLine spec, an OpLine instruction must be applied to
the instructions physically following it up to the first occurrence
of the next end of block, the next OpLine instruction, or the next
OpNoLine instruction.

```
OpLine %file 0 0
OpNoLine
OpLine %file 1 1
OpStore %foo %int_1
%value = OpLoad %int %foo
OpLine %file 2 2
```

For the above code, the current spirv-opt keeps three line
instructions `OpLine %file 0 0`, `OpNoLine`, and `OpLine %file 1 1`
in `std::vector<Instruction> dbg_line_insts_` of Instruction class
for `OpStore %foo %int_1`. It does not put any line instruction to
`std::vector<Instruction> dbg_line_insts_` of
`%value = OpLoad %int %foo` even though `OpLine %file 1 1` must be
applied to `%value = OpLoad %int %foo` based on the spec.

This results in the missing line information for
`%value = OpLoad %int %foo` while each spirv-opt pass optimizes the
code. We have to put `OpLine %file 1 1` to
`std::vector<Instruction> dbg_line_insts_` of
both `%value = OpLoad %int %foo` and `OpStore %foo %int_1`.

This commit conducts the line instruction propagation and skips
emitting the eliminated line instructions at the end, which are the same
with PropagateLineInfoPass and RedundantLineInfoElimPass. This
commit removes PropagateLineInfoPass and RedundantLineInfoElimPass.

KhronosGroup/glslang#2440 is a related PR that stop using
PropagateLineInfoPass and RedundantLineInfoElimPass from glslang.
When the code in this PR applied, the glslang tests will pass.
@jaebaek
Copy link
Copy Markdown
Contributor Author

jaebaek commented Oct 29, 2020

@johnkslang Hi, JohK. I updated the spirv-tools hash in known_good.json. Could you please review this PR?

jaebaek added a commit to jaebaek/SPIRV-Tools that referenced this pull request Oct 30, 2020
Removing PropagateLineInfoPass and RedundantLineInfoElimPass from
56d0f50 makes unit tests of many open source projects fail.
It will happen before submitting this glslang PR
KhronosGroup/glslang#2440. This commit will be
git-reverted after merging the glslang PR.
jaebaek added a commit to KhronosGroup/SPIRV-Tools that referenced this pull request Oct 30, 2020
Removing PropagateLineInfoPass and RedundantLineInfoElimPass from
56d0f50 makes unit tests of many open source projects fail.
It will happen before submitting this glslang PR
KhronosGroup/glslang#2440. This commit will be
git-reverted after merging the glslang PR.
@johnkslang johnkslang merged commit ed8bd04 into KhronosGroup:master Nov 2, 2020
@jaebaek jaebaek deleted the opline branch November 2, 2020 18:32
jaebaek added a commit to jaebaek/SPIRV-Tools that referenced this pull request Nov 2, 2020
…ronosGroup#4004)"

This reverts commit f7da527.

- The commit f7da527 added EmptyPass that temporarily handles glslang
failure caused by missing PropagateLineInfoPass and
RedundantLineInfoElimPass. Now glslang does not use PropagateLineInfoPass
and RedundantLineInfoElimPass (KhronosGroup/glslang#2440).
We can revert f7da527.
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

Successfully merging this pull request may close these issues.

2 participants