Skip to content

[AMDGPU] Add test for GCNRegPressure tracker bug #73786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 30, 2023
Merged

[AMDGPU] Add test for GCNRegPressure tracker bug #73786

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3d36440
[AMDGPU] Add test for GCNRegPressure tracker bug
piotrAMD Nov 29, 2023
c294259
Merge remote-tracking branch 'origin/main'
piotrAMD Nov 30, 2023
10391e0
Add missing live-throughs
piotrAMD Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 135 additions & 0 deletions llvm/test/CodeGen/AMDGPU/regpressure_printer.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,3 +531,138 @@ body: |
%1:vgpr_32 = V_MOV_B32_e32 %0, implicit $exec
S_NOP 0, implicit %1
...
---
name: movrel
tracksRegLiveness: true
body: |
; RPU-LABEL: name: movrel
; RPU: bb.0:
; RPU-NEXT: Live-in:
; RPU-NEXT: SGPR VGPR
; RPU-NEXT: 0 0
; RPU-NEXT: 0 0 $sgpr0 = COPY $sgpr1
; RPU-NEXT: 0 0
; RPU-NEXT: 0 0 $sgpr2_sgpr3 = S_GETPC_B64
; RPU-NEXT: 0 0
; RPU-NEXT: 0 0 $sgpr1 = COPY killed $sgpr3
; RPU-NEXT: 0 0
; RPU-NEXT: 0 0 $sgpr0_sgpr1_sgpr2_sgpr3 = S_LOAD_DWORDX4_IMM $sgpr0_sgpr1, 0, 0
; RPU-NEXT: 0 0
; RPU-NEXT: 0 0 $sgpr0 = S_BUFFER_LOAD_DWORD_IMM $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0
; RPU-NEXT: 0 0
; RPU-NEXT: 0 0 undef %0.sub5:vreg_512 = V_MOV_B32_e32 5, implicit $exec
; RPU-NEXT: 0 0
; RPU-NEXT: 0 0 S_CMP_GT_U32 $sgpr0, 15, implicit-def $scc
; RPU-NEXT: 0 0
; RPU-NEXT: 0 0 S_CBRANCH_SCC1 %bb.2, implicit $scc
; RPU-NEXT: 0 0
; RPU-NEXT: 0 0 S_BRANCH %bb.1
; RPU-NEXT: 0 0
; RPU-NEXT: Live-out:
; RPU-NEXT: Live-thr:
; RPU-NEXT: 0 0
; RPU-NEXT: bb.1:
; RPU-NEXT: Live-in:
; RPU-NEXT: SGPR VGPR
; RPU-NEXT: 0 0
; RPU-NEXT: 0 1 undef %0.sub5:vreg_512 = V_MOV_B32_e32 5, implicit $exec
; RPU-NEXT: 0 1
; RPU-NEXT: 0 1 $m0 = S_MOV_B32 killed $sgpr0
; RPU-NEXT: 0 1
; RPU-NEXT: 0 1 %0:vreg_512 = V_INDIRECT_REG_WRITE_MOVREL_B32_V16 %0:vreg_512(tied-def 0), 42, 3, implicit $m0, implicit $exec
; RPU-NEXT: 0 1
; RPU-NEXT: Live-out: %0:0000000000000C00
; RPU-NEXT: Live-thr:
; RPU-NEXT: 0 0
; RPU-NEXT: bb.2:
; RPU-NEXT: Live-in: %0:0000000000000C00
; RPU-NEXT: SGPR VGPR
; RPU-NEXT: 0 1
; RPU-NEXT: 0 1 %1:vgpr_32 = V_CVT_F32_UBYTE0_e64 %0.sub5:vreg_512, 0, 0, implicit $exec
Copy link
Contributor

@vpykhtin vpykhtin Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We need to agree on how we count such cases.

The problem is that %0 is fully defined by V_INDIRECT_REG_WRITE_MOVREL_B32_V16 but only sub5 of it is used. In general this means that regalloc need to allocate full vreg_512 anyway but the unused lanes can be allocated for other needs though this is not the case here.

This makes tracking more complicated if we start model what regalloc would do. The conservative approach can be to ignore lanes at all after the GCNRewritePartialRegUses pass is enabled because after this pass is guaranteed we have only fully defined or used registers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wonder what others think, but for me what RPD reports seems intuitively correct - the RP gets the increment from the movrel just for the instruction (and that should contribute to the max pressure).

Unless I misunderstood the suggestion I do not think this is related to GCNRewritePartialRegUses. The movrel instruction is kind of special because of the indirection. It can't just operate on %0.sub5.

Copy link
Contributor

Choose a reason for hiding this comment

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

RPD is accounting for the whole vreg_512 "at" the instruction level though. If some of the lanes are reused after the instruction they should be already decremented from the pressure. It looks like RPD approach is more correct here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I misunderstood the suggestion I do not think this is related to GCNRewritePartialRegUses.

GCNRewritePartialRegUses is irrelevant to this case indeed, I just thought about a conservative way of register pressure accounting when we account for the whole reg always, but we can do better than that as RPD does.

The movrel instruction is kind of special because of the indirection. It can't just operate on %0.sub5.

Sorry I don't really know how it works but I believe
%0:vreg_512 = V_INDIRECT_REG_WRITE_MOVREL_B32_V16 %0:vreg_512(tied-def 0)
models correctly what is does, that is it fully defines %0:vreg_512 on output, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

@piotrAMD are you going to fix this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with all of this - what RPD is doing seems correct.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@piotrAMD are you going to fix this?

Yes, happy to work on the fix when I am done with the task I am currently working on.

; RPU-NEXT: 0 1
; RPU-NEXT: 0 1 EXP_DONE 0, %1:vgpr_32, undef %2:vgpr_32, undef %3:vgpr_32, undef %4:vgpr_32, -1, 0, 1, implicit $exec
; RPU-NEXT: 0 0
; RPU-NEXT: 0 0 S_ENDPGM 0
; RPU-NEXT: 0 0
; RPU-NEXT: Live-out:
; RPU-NEXT: Live-thr:
; RPU-NEXT: 0 0
;
; RPD-LABEL: name: movrel
; RPD: bb.0:
; RPD-NEXT: Live-in:
; RPD-NEXT: SGPR VGPR
; RPD-NEXT: 0 0
; RPD-NEXT: 0 0 $sgpr0 = COPY $sgpr1
; RPD-NEXT: 0 0
; RPD-NEXT: 0 0 $sgpr2_sgpr3 = S_GETPC_B64
; RPD-NEXT: 0 0
; RPD-NEXT: 0 0 $sgpr1 = COPY killed $sgpr3
; RPD-NEXT: 0 0
; RPD-NEXT: 0 0 $sgpr0_sgpr1_sgpr2_sgpr3 = S_LOAD_DWORDX4_IMM $sgpr0_sgpr1, 0, 0
; RPD-NEXT: 0 0
; RPD-NEXT: 0 0 $sgpr0 = S_BUFFER_LOAD_DWORD_IMM $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0
; RPD-NEXT: 0 0
; RPD-NEXT: 0 1 undef %0.sub5:vreg_512 = V_MOV_B32_e32 5, implicit $exec
; RPD-NEXT: 0 1
; RPD-NEXT: 0 1 S_CMP_GT_U32 $sgpr0, 15, implicit-def $scc
; RPD-NEXT: 0 1
; RPD-NEXT: 0 1 S_CBRANCH_SCC1 %bb.2, implicit $scc
; RPD-NEXT: 0 1
; RPD-NEXT: 0 1 S_BRANCH %bb.1
; RPD-NEXT: 0 1
; RPD-NEXT: Live-out: %0:0000000000000C00
; RPD-NEXT: mis LIS:
; RPD-NEXT: %0:L0000000000000C00 isn't found in LIS reported set
; RPD-NEXT: Live-thr:
; RPD-NEXT: 0 0
; RPD-NEXT: bb.1:
; RPD-NEXT: Live-in:
; RPD-NEXT: SGPR VGPR
; RPD-NEXT: 0 0
; RPD-NEXT: 0 1 undef %0.sub5:vreg_512 = V_MOV_B32_e32 5, implicit $exec
; RPD-NEXT: 0 1
; RPD-NEXT: 0 1 $m0 = S_MOV_B32 killed $sgpr0
; RPD-NEXT: 0 1
; RPD-NEXT: 0 16 %0:vreg_512 = V_INDIRECT_REG_WRITE_MOVREL_B32_V16 %0:vreg_512(tied-def 0), 42, 3, implicit $m0, implicit $exec
; RPD-NEXT: 0 1
; RPD-NEXT: Live-out: %0:0000000000000C00
; RPD-NEXT: Live-thr:
; RPD-NEXT: 0 0
; RPD-NEXT: bb.2:
; RPD-NEXT: Live-in: %0:0000000000000C00
; RPD-NEXT: SGPR VGPR
; RPD-NEXT: 0 1
; RPD-NEXT: 0 2 %1:vgpr_32 = V_CVT_F32_UBYTE0_e64 %0.sub5:vreg_512, 0, 0, implicit $exec
; RPD-NEXT: 0 1
; RPD-NEXT: 0 1 EXP_DONE 0, %1:vgpr_32, undef %2:vgpr_32, undef %3:vgpr_32, undef %4:vgpr_32, -1, 0, 1, implicit $exec
; RPD-NEXT: 0 0
; RPD-NEXT: 0 0 S_ENDPGM 0
; RPD-NEXT: 0 0
; RPD-NEXT: Live-out:
; RPD-NEXT: Live-thr:
; RPD-NEXT: 0 0
bb.0:
liveins: $sgpr1
$sgpr0 = COPY $sgpr1
$sgpr2_sgpr3 = S_GETPC_B64
$sgpr1 = COPY killed $sgpr3
$sgpr0_sgpr1_sgpr2_sgpr3 = S_LOAD_DWORDX4_IMM killed $sgpr0_sgpr1, 0, 0
$sgpr0 = S_BUFFER_LOAD_DWORD_IMM killed $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0
undef %47.sub5:vreg_512 = V_MOV_B32_e32 5, implicit $exec
S_CMP_GT_U32 $sgpr0, 15, implicit-def $scc
S_CBRANCH_SCC1 %bb.2, implicit $scc
S_BRANCH %bb.1

bb.1:
liveins: $sgpr0
undef %47.sub5:vreg_512 = V_MOV_B32_e32 5, implicit $exec
$m0 = S_MOV_B32 killed $sgpr0
%47:vreg_512 = V_INDIRECT_REG_WRITE_MOVREL_B32_V16 %47:vreg_512, 42, 3, implicit $m0, implicit $exec

bb.2:

%49:vgpr_32 = V_CVT_F32_UBYTE0_e64 %47.sub5:vreg_512, 0, 0, implicit $exec
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
...