KhronosGroup / KhronosGroup/SPIRV-Tools

Fold-spec-constant pass creates duplicate constants.

Open
#1,723 0 comments 0 reactions 0 assignees View on GitHub
component:optimization enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.