Skip to content

[WaveTransform] Handle SI_WAVE_CF_EDGE in SGPR hazard wait merging - #3813

Closed
cdevadas wants to merge 1 commit into
amd-feature/wave-transformfrom
public/amd/dev/cdevadas/wave-transform/incorrect-insertion-pt
Closed

[WaveTransform] Handle SI_WAVE_CF_EDGE in SGPR hazard wait merging#3813
cdevadas wants to merge 1 commit into
amd-feature/wave-transformfrom
public/amd/dev/cdevadas/wave-transform/incorrect-insertion-pt

Conversation

@cdevadas

@cdevadas cdevadas commented Aug 7, 2026

Copy link
Copy Markdown

The AMDGPUWaitSGPRHazards pass's runWaitMerging function skips meta instructions and only treats isBranch/isCall/isReturn as control-flow boundaries. SI_WAVE_CF_EDGE is a meta terminator that is none of these, so the pass would skip over it and move S_WAITCNT_DEPCTR past it to before the following S_CBRANCH_EXECZ. This placed a non-terminator instruction between two terminators, causing a machine verifier error.

Fix by teaching runWaitMerging to not skip SI_WAVE_CF_EDGE despite it being a meta instruction, and to treat it as a control-flow boundary alongside branches, calls, and returns.

Note: This is a temporary workaround. The handling should be done differently before upstreaming this feature.

This also removes the XFAIL from
divergence-divergent-i1-used-outside-loop.ll and regenerates its CHECK lines.

The AMDGPUWaitSGPRHazards pass's runWaitMerging function skips meta
instructions and only treats isBranch/isCall/isReturn as control-flow
boundaries. SI_WAVE_CF_EDGE is a meta terminator that is none of these,
so the pass would skip over it and move S_WAITCNT_DEPCTR past it to
before the following S_CBRANCH_EXECZ. This placed a non-terminator
instruction between two terminators, causing a machine verifier error.

Fix by teaching runWaitMerging to not skip SI_WAVE_CF_EDGE despite it
being a meta instruction, and to treat it as a control-flow boundary
alongside branches, calls, and returns.

Note: This is a temporary workaround. The handling should be done
differently before upstreaming this feature.

This also removes the XFAIL from
divergence-divergent-i1-used-outside-loop.ll and regenerates its CHECK
lines.
; GFX1100-W64-NEXT: v_cmp_ne_u32_e64 s[0:1], 0, v0
; GFX1100-W64-NEXT: s_mov_b32 s6, 0
; GFX1100-W64-NEXT: s_mov_b64 s[8:9], -1
; GFX1100-W64-NEXT: s_mov_b64 s[12:13], -1

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the test changes are not due to this patch. This was earlier XFAILed during the previous merge from amd-staging commit due to this error. Now I regenrated their check patterns for the latest changes.

@cdevadas

cdevadas commented Aug 7, 2026

Copy link
Copy Markdown
Author

I don't particularly like this fix. But we need a similar custom fix at the moment to avoid this error. The SI_WAVE_CF_EDGE should be a Meta instruction. Removing the Meta field from it causes some problems at the moment. So I custom-added these instructions in the two conditional checks in AMDGPUWaitSGPRHazards.

@rocm-cciapp

rocm-cciapp Bot commented Aug 7, 2026

Copy link
Copy Markdown

@cdevadas

Copy link
Copy Markdown
Author

Closing this infavor of upstream PR llvm#214935

@cdevadas cdevadas closed this Aug 13, 2026
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.

3 participants