KhronosGroup / KhronosGroup/SPIRV-Tools
Fold-spec-constant pass creates duplicate constants.
- Dominant language
- C++
- Stars
- 1.4k
- Forks
- 709
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 28
Description
The pass that folds the spec constants will call `BuildInstructionAndAddToModule` each time it needs to find a constant. This causes duplicate constants, and the test cases do not have a single correct output.
I would be nice to replace all of the calls to `BuildInstructionAndAddToModule` in fold_spec_constant_op_and_composite_pass.cpp by calls to `GetDefiningInstruction`. The time consuming part is that almost all of the test will need to be updated. For many of them, the current testing strategy will not be viable.
One example if a test that needs to be redesigned is case 0 in `CompositeExtract` case. It defines `%r` as a spec constant. It will fold to some other id, but there is no way to tie `%r` with the other value unless we look at the OpName.
We will also have a problem because an id could have multiple OpNames because we do not kill the name before calling `ReplaceAllUses`.
Contributor guide
Research direction
Start in fold_spec_constant_op_and_composite_pass.cpp and inspect each BuildInstructionAndAddToModule call alongside GetDefiningInstruction. Then review the fold-spec-constant tests, especially CompositeExtract case 0, to determine which expectations rely on generated IDs or OpName entries. Done means the pass no longer creates duplicate constants and the affected tests have a single correct output.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers, testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100