llvm / llvm/llvm-project

[AMDGPU][gfx1250] `SIPreEmitPeephole` branch removal affects `S_WAIT_XCNT` placement

Open
#213,394 2 comments 0 reactions 1 assignee Claimed by @Pierre-vh View on GitHub
backend:AMDGPU
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

## Summary

A normal gfx1250 compilation runs `si-insert-waitcnts` and then, a few passes later, `si-pre-emit-peephole`. On the example below, the program that comes out of that pipeline contains a `GLOBAL_STORE_DWORDX4` whose address register is overwritten by the next instruction, with no `S_WAIT_XCNT` in between.

If `si-insert-waitcnts` is then run a second time on that same final output, it inserts an `S_WAIT_XCNT 0` before the overwrite. I think this new `S_WAIT_XCNT 0` is correct.

## Reproducer

`xcnt-example.mir` — four instructions across three basic blocks:

```mir
--- |
target triple = "amdgcn-amd-amdhsa"

define amdgpu_kernel void @xcnt_execz_example() #0 {
entry:
unreachable
}

attributes #0 = { "target-cpu"="gfx1250" "target-features"="+wavefrontsize32,-wavefrontsize64" }
...
---
name: xcnt_execz_example
tracksRegLiveness: true
machineFunctionInfo:
isEntryFunction: true
body: |
bb.0:
successors: %bb.1, %bb.2
liveins: $vgpr2, $vgpr3, $vgpr18, $vgpr19, $vgpr20, $vgpr21
GLOBAL_STORE_DWORDX4 killed $vgpr2_vgpr3, killed $vgpr18_vgpr19_vgpr20_vgpr21, 0, 0, implicit $exec
S_CBRANCH_EXECZ %bb.2, implicit $exec

bb.1:
successors: %bb.2
$vgpr2 = V_MOV_B32_e32 0, implicit $exec

bb.2:
S_ENDPGM 0
...
```

## Steps to Reproduce

```sh
llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1250 -run-pass=si-insert-waitcnts xcnt-example.mir -o s1.mir
llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1250 -run-pass=si-pre-emit-peephole s1.mir -o s2.mir
llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1250 -run-pass=si-insert-waitcnts s2.mir -o s3.mir
```

`s2.mir` is what the current pipeline emits and there are no `S_WAIT_XCNT`s. After running the insert-waitcnt pass again, it inserts a new one in `s3.mir`. I believe `s3.mir` is more correct because it ensures the wait before `$vgpr2` is overwritten.

Note that running the pass more times does not make anymore changes.

## Build notes

* commit 7ef0ca2b13
* Release build with assertions, AMDGPU, target gfx1250

## Credit

Credit goes to @kuhar for finding the discrepancy.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.