-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[AMDGPU] Fold into uses of splat REG_SEQUENCEs through COPYs. #145691
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.
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
Conversation
|
@llvm/pr-subscribers-backend-amdgpu Author: Ivan Kosarev (kosarev) ChangesFull diff: https://github.com/llvm/llvm-project/pull/145691.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
index 8299c0dce1870..2d6afcbfb9447 100644
--- a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
@@ -1036,11 +1036,18 @@ void SIFoldOperandsImpl::foldOperand(
// Grab the use operands first
SmallVector<MachineOperand *, 4> UsesToProcess(
llvm::make_pointer_range(MRI->use_nodbg_operands(RegSeqDstReg)));
- for (auto *RSUse : UsesToProcess) {
+ for (unsigned I = 0; I != UsesToProcess.size(); ++I) {
+ MachineOperand *RSUse = UsesToProcess[I];
MachineInstr *RSUseMI = RSUse->getParent();
unsigned OpNo = RSUseMI->getOperandNo(RSUse);
if (SplatVal) {
+ if (RSUseMI->isCopy()) {
+ Register DstReg = RSUseMI->getOperand(0).getReg();
+ append_range(UsesToProcess,
+ make_pointer_range(MRI->use_nodbg_operands(DstReg)));
+ continue;
+ }
if (MachineOperand *Foldable =
tryFoldRegSeqSplat(RSUseMI, OpNo, SplatVal, SplatRC)) {
appendFoldCandidate(FoldList, RSUseMI, OpNo, Foldable);
diff --git a/llvm/test/CodeGen/AMDGPU/packed-fp32.ll b/llvm/test/CodeGen/AMDGPU/packed-fp32.ll
index bef38c1a65ef8..4db3f2189bfc3 100644
--- a/llvm/test/CodeGen/AMDGPU/packed-fp32.ll
+++ b/llvm/test/CodeGen/AMDGPU/packed-fp32.ll
@@ -2155,11 +2155,8 @@ define amdgpu_kernel void @fadd_fadd_fsub_0(<2 x float> %arg) {
; GFX90A-GISEL-LABEL: fadd_fadd_fsub_0:
; GFX90A-GISEL: ; %bb.0: ; %bb
; GFX90A-GISEL-NEXT: s_load_dwordx2 s[0:1], s[4:5], 0x24
-; GFX90A-GISEL-NEXT: s_mov_b32 s2, 0
-; GFX90A-GISEL-NEXT: s_mov_b32 s3, s2
-; GFX90A-GISEL-NEXT: v_pk_mov_b32 v[0:1], s[2:3], s[2:3] op_sel:[0,1]
; GFX90A-GISEL-NEXT: s_waitcnt lgkmcnt(0)
-; GFX90A-GISEL-NEXT: v_pk_add_f32 v[0:1], s[0:1], v[0:1]
+; GFX90A-GISEL-NEXT: v_pk_add_f32 v[0:1], s[0:1], 0
; GFX90A-GISEL-NEXT: v_mov_b32_e32 v0, v1
; GFX90A-GISEL-NEXT: v_pk_add_f32 v[0:1], v[0:1], 0
; GFX90A-GISEL-NEXT: v_mov_b32_e32 v2, s0
@@ -2170,11 +2167,8 @@ define amdgpu_kernel void @fadd_fadd_fsub_0(<2 x float> %arg) {
; GFX942-GISEL-LABEL: fadd_fadd_fsub_0:
; GFX942-GISEL: ; %bb.0: ; %bb
; GFX942-GISEL-NEXT: s_load_dwordx2 s[0:1], s[4:5], 0x24
-; GFX942-GISEL-NEXT: s_mov_b32 s2, 0
-; GFX942-GISEL-NEXT: s_mov_b32 s3, s2
-; GFX942-GISEL-NEXT: v_mov_b64_e32 v[0:1], s[2:3]
; GFX942-GISEL-NEXT: s_waitcnt lgkmcnt(0)
-; GFX942-GISEL-NEXT: v_pk_add_f32 v[0:1], s[0:1], v[0:1]
+; GFX942-GISEL-NEXT: v_pk_add_f32 v[0:1], s[0:1], 0
; GFX942-GISEL-NEXT: s_nop 0
; GFX942-GISEL-NEXT: v_mov_b32_e32 v0, v1
; GFX942-GISEL-NEXT: v_pk_add_f32 v[0:1], v[0:1], 0
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only test change is one globalisel case, so that's a hint this is just hiding a missed optimization that should have happened earlier in the pipeline. Do you have another example where this is useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs mir test if we're going through with this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
kosarev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebased.
| unsigned OpNo = RSUseMI->getOperandNo(RSUse); | ||
|
|
||
| if (SplatRC) { | ||
| if (RSUseMI->isCopy()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this could be isFoldableCopy(), but I hesitate to use it without test coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed elsewhere.
|
Ping. |
3 similar comments
|
Ping. |
|
Ping. |
|
Ping. |
|
Ping. |
1 similar comment
|
Ping. |
|
@arsenm Ping. |
rampitec
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a nit.
| ; CHECK-NEXT: [[DEF:%[0-9]+]]:sreg_64 = IMPLICIT_DEF | ||
| ; CHECK-NEXT: [[DEF1:%[0-9]+]]:sreg_64_xexec = IMPLICIT_DEF | ||
| ; CHECK-NEXT: [[V_PK_ADD_F32_:%[0-9]+]]:vreg_64_align2 = nofpexcept V_PK_ADD_F32 8, [[DEF1]], 8, 0, 0, 0, 0, 0, 0, implicit $mode, implicit $exec | ||
| %15:sreg_32 = S_MOV_B32 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Registers could be renumbered here to start from 0.
No description provided.