KhronosGroup / SPIRV-LLVM-Translator

A tool and a library for bi-directional translation between SPIR-V and LLVM IR
Other
468 stars 209 forks source link

Skip adding decorations for OpForward #2529

Closed svenvh closed 4 months ago

svenvh commented 4 months ago

When a temporary OpForward instruction is needed during translation to SPIR-V, do not add the decorations yet, as that would result in duplicate decorations when the actual instruction is visited and the OpForward is replaced by a real SPIR-V instruction.

The SPIR-V Validator has recently started checking for duplicate decorations; this fixes some but not all issues arising from the new checks.

Contributes to https://github.com/KhronosGroup/SPIRV-LLVM-Translator/issues/2509 .

svenvh commented 4 months ago

Should we enable spirv-val checks in some of our tests if they have such a problem? Or even it'd be nice to see a separate test for the fix.

With the latest version of spirv-val the test/transcoding/OpPhi_ArgumentsPlaceholders.ll test currently fails validation, see #2509. This PR fixes that test.

We don't see the failure in GitHub CI yet because it uses an older version of spirv-val.