[llvm] [AMDGPU] Fix GCUpwardRPTracker. (PR #74328)

Valery Pykhtin via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 6 10:59:17 PST 2023


================
@@ -666,3 +625,70 @@ body: |
     EXP_DONE 0, %49:vgpr_32, undef %51:vgpr_32, undef %53:vgpr_32, undef %55:vgpr_32, -1, 0, 1, implicit $exec
     S_ENDPGM 0
 ...
+---
+name: early_clobber_def_used_on_rhs
+registers:
+  - { id: 0, class: vgpr_32 }
+body: |
+  ; RPU-LABEL: name: early_clobber_def_used_on_rhs
+  ; RPU: bb.0:
+  ; RPU-NEXT:   Live-in:
+  ; RPU-NEXT:   SGPR  VGPR
+  ; RPU-NEXT:   0     0
+  ; RPU-NEXT:   0     1      dead %3:vgpr_32 = COPY $vgpr0
+  ; RPU-NEXT:   0     0
+  ; RPU-NEXT:   0     1      early-clobber %2:vgpr_32 = COPY %0:vgpr_32
----------------
vpykhtin wrote:

Actually this isn't MIR parser, this is done by _LiveIntervals_ analysis.

Firstly, I forgot to add `tracksRegLiveness: true` line in the test - _LiveIntervals_  requires that. But even with the flag, the verifier fails like this:

```
********** INTERVALS **********
VGPR0_LO16 [0B,16r:0) 0 at 0B-phi
VGPR0_HI16 [0B,16r:0) 0 at 0B-phi
%0 [32e,48r:0) 0 at 32e  weight:0.000000e+00
%1 [16r,16d:0) 0 at 16r  weight:0.000000e+00
RegMasks:
********** MACHINEINSTRS **********
# Machine code for function early_clobber_def_used_on_rhs: NoPHIs, TracksLiveness

0B	bb.0:
	  liveins: $vgpr0
16B	  dead %1:vgpr_32 = COPY $vgpr0
32B	  early-clobber %0:vgpr_32 = COPY %0:vgpr_32
48B	  S_NOP 0, implicit %0:vgpr_32

# End machine code for function early_clobber_def_used_on_rhs.

*** Bad machine code: No live segment at use ***
- function:    early_clobber_def_used_on_rhs
- basic block: %bb.0  (0x1f9fb80) [0B;64B)
- instruction: 32B	early-clobber %0:vgpr_32 = COPY %0:vgpr_32
- operand 1:   %0:vgpr_32
- liverange:   [32e,48r:0) 0 at 32e
- v. register: %0
- at:          32B
```
Verifier complains that there is no live segment at 32B slot index, that is at the entry to 32 instruction. _LiveIntervals_ correctly determined that the value produced at 16R is dead because it's clobbered by the def at 32E before the use at 32R. It renamed %0 at 16R to %1 and marked it as dead but I think it should also mark the %0 use at 32R as 'undef' for this IR to be valid. If I add 'undef' flag there manually everything works fine:
```
bb.0:
    Live-in: 
    SGPR  VGPR
    0     0    
    0     1      dead %1:vgpr_32 = COPY $vgpr0
    0     0    
    0     1      early-clobber %0:vgpr_32 = COPY undef %0:vgpr_32
    0     1    
    0     1      S_NOP 0, implicit %0:vgpr_32
    0     0    
```
So I think current implementation of _GCNUpwardRPTracker_ is correct if it works on correct IR. _LiveIntervals_ should be fixed.



https://github.com/llvm/llvm-project/pull/74328


More information about the llvm-commits mailing list