[llvm] [AMDGPU] Fix mode register intersection across predecessors (PR #222244)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 8 22:51:36 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-amdgpu
Author: Arseniy Obolenskiy (aobolensk)
<details>
<summary>Changes</summary>
The merge loop tested a stale ExitSet flag from an earlier visit, so each known predecessor overwrote the entry mode instead of intersecting into it
---
Full diff: https://github.com/llvm/llvm-project/pull/222244.diff
3 Files Affected:
- (modified) llvm/lib/Target/AMDGPU/SIModeRegister.cpp (+22-27)
- (modified) llvm/test/CodeGen/AMDGPU/mode-register-fpconstrain.ll (+51)
- (modified) llvm/test/CodeGen/AMDGPU/mode-register.mir (+56)
``````````diff
diff --git a/llvm/lib/Target/AMDGPU/SIModeRegister.cpp b/llvm/lib/Target/AMDGPU/SIModeRegister.cpp
index dbe27a8030d4e..6bb99944facfc 100644
--- a/llvm/lib/Target/AMDGPU/SIModeRegister.cpp
+++ b/llvm/lib/Target/AMDGPU/SIModeRegister.cpp
@@ -372,37 +372,32 @@ void SIModeRegister::processBlockPhase2(MachineBasicBlock &MBB,
// Mask bits (which represent the Mode bits with a known value) can only be
// added by explicit SETREG instructions or the initial default value -
// the intersection process may remove Mask bits.
- // If we find a predecessor that has not yet had an exit value determined
- // (this can happen for example if a block is its own predecessor) we defer
- // use of that value as the Mask will be all zero, and we will revisit this
- // block again later (unless the only predecessor without an exit value is
- // this block).
- MachineBasicBlock::pred_iterator P = MBB.pred_begin(), E = MBB.pred_end();
- MachineBasicBlock &PB = *(*P);
- unsigned PredBlock = PB.getNumber();
- if ((ThisBlock == PredBlock) && (std::next(P) == E)) {
- BlockInfo[ThisBlock]->Pred = DefaultStatus;
+ BlockData &Info = *BlockInfo[ThisBlock];
+ bool SelfPredPending = false;
+ // An entry block is also entered from the function entry, so the default
+ // applies even when it has predecessors.
+ if (MBB.isEntryBlock()) {
+ Info.Pred = DefaultStatus;
ExitSet = true;
- } else if (BlockInfo[PredBlock]->ExitSet) {
- BlockInfo[ThisBlock]->Pred = BlockInfo[PredBlock]->Exit;
- ExitSet = true;
- } else if (PredBlock != ThisBlock)
- RevisitRequired = true;
-
- for (P = std::next(P); P != E; P = std::next(P)) {
- MachineBasicBlock *Pred = *P;
+ }
+ for (MachineBasicBlock *Pred : MBB.predecessors()) {
unsigned PredBlock = Pred->getNumber();
- if (BlockInfo[PredBlock]->ExitSet) {
- if (BlockInfo[ThisBlock]->ExitSet) {
- BlockInfo[ThisBlock]->Pred =
- BlockInfo[ThisBlock]->Pred.intersect(BlockInfo[PredBlock]->Exit);
- } else {
- BlockInfo[ThisBlock]->Pred = BlockInfo[PredBlock]->Exit;
- }
+ const BlockData &PredInfo = *BlockInfo[PredBlock];
+ if (!PredInfo.ExitSet) {
+ if (PredBlock == ThisBlock)
+ SelfPredPending = true;
+ else
+ RevisitRequired = true;
+ } else if (ExitSet) {
+ Info.Pred = Info.Pred.intersect(PredInfo.Exit);
+ } else {
+ Info.Pred = PredInfo.Exit;
ExitSet = true;
- } else if (PredBlock != ThisBlock)
- RevisitRequired = true;
+ }
}
+ // ExitSet gating stops an unreachable self-only block from requeuing
+ // forever, as its exit never becomes known.
+ RevisitRequired |= SelfPredPending && ExitSet;
}
Status TmpStatus =
BlockInfo[ThisBlock]->Pred.merge(BlockInfo[ThisBlock]->Change);
diff --git a/llvm/test/CodeGen/AMDGPU/mode-register-fpconstrain.ll b/llvm/test/CodeGen/AMDGPU/mode-register-fpconstrain.ll
index 174435795c5ef..a7932eca0e4ba 100644
--- a/llvm/test/CodeGen/AMDGPU/mode-register-fpconstrain.ll
+++ b/llvm/test/CodeGen/AMDGPU/mode-register-fpconstrain.ll
@@ -30,6 +30,57 @@ entry:
ret double %val
}
+; The entry fadd is load-bearing: it makes every loop predecessor exit stable
+; from the start, which is what hid the loop exit from phase-2 intersection.
+
+define amdgpu_kernel void @loop_carried_round_mode(ptr addrspace(1) %out, double %a, double %b, i32 %n) {
+; GCN-LABEL: loop_carried_round_mode:
+; GCN: ; %bb.0: ; %entry
+; GCN-NEXT: s_load_dwordx2 s[4:5], s[8:9], 0x10
+; GCN-NEXT: s_load_dwordx4 s[0:3], s[8:9], 0x0
+; GCN-NEXT: s_load_dword s6, s[8:9], 0x18
+; GCN-NEXT: v_mov_b32_e32 v2, 0
+; GCN-NEXT: s_mov_b32 s7, 0
+; GCN-NEXT: s_waitcnt lgkmcnt(0)
+; GCN-NEXT: v_mov_b32_e32 v0, s4
+; GCN-NEXT: v_mov_b32_e32 v1, s5
+; GCN-NEXT: v_add_f64 v[0:1], s[2:3], v[0:1]
+; GCN-NEXT: global_store_dwordx2 v2, v[0:1], s[0:1]
+; GCN-NEXT: s_waitcnt vmcnt(0)
+; GCN-NEXT: v_mov_b32_e32 v0, s2
+; GCN-NEXT: v_mov_b32_e32 v1, s3
+; GCN-NEXT: .LBB2_1: ; %loop
+; GCN-NEXT: ; =>This Inner Loop Header: Depth=1
+; GCN-NEXT: s_setreg_imm32_b32 hwreg(HW_REG_MODE, 2, 1), 0
+; GCN-NEXT: v_add_f64 v[0:1], v[0:1], s[4:5]
+; GCN-NEXT: s_add_i32 s7, s7, 1
+; GCN-NEXT: s_cmp_lt_i32 s7, s6
+; GCN-NEXT: s_setreg_imm32_b32 hwreg(HW_REG_MODE, 2, 2), 1
+; GCN-NEXT: v_cvt_f32_f64_e32 v3, v[0:1]
+; GCN-NEXT: global_store_dword v2, v3, s[0:1]
+; GCN-NEXT: s_waitcnt vmcnt(0)
+; GCN-NEXT: s_cbranch_scc1 .LBB2_1
+; GCN-NEXT: ; %bb.2: ; %exit
+; GCN-NEXT: s_endpgm
+entry:
+ %e = fadd double %a, %b
+ store volatile double %e, ptr addrspace(1) %out
+ br label %loop
+
+loop:
+ %i = phi i32 [ 0, %entry ], [ %i.next, %loop ]
+ %acc = phi double [ %a, %entry ], [ %sum, %loop ]
+ %sum = fadd double %acc, %b
+ %t = call float @llvm.fptrunc.round.f32.f64(double %sum, metadata !"round.upward")
+ store volatile float %t, ptr addrspace(1) %out
+ %i.next = add i32 %i, 1
+ %cc = icmp slt i32 %i.next, %n
+ br i1 %cc, label %loop, label %exit
+
+exit:
+ ret void
+}
+
declare void @llvm.amdgcn.s.setreg(i32 immarg, i32)
declare double @llvm.experimental.constrained.fadd.f64(double, double, metadata, metadata)
diff --git a/llvm/test/CodeGen/AMDGPU/mode-register.mir b/llvm/test/CodeGen/AMDGPU/mode-register.mir
index 1dd6499ca2733..24611e4238257 100644
--- a/llvm/test/CodeGen/AMDGPU/mode-register.mir
+++ b/llvm/test/CodeGen/AMDGPU/mode-register.mir
@@ -510,3 +510,59 @@ body: |
bb.2:
S_ENDPGM 0
...
+---
+# Predecessor exits used to overwrite rather than intersect, so the last
+# predecessor won and the RTN restore in bb.3 was dropped.
+# CHECK-LABEL: name: join_pred_intersect
+# CHECK-LABEL: bb.3:
+# CHECK: S_SETREG_IMM32_B32 0, 2177
+# CHECK-NEXT: V_ADD_F64_e64
+
+name: join_pred_intersect
+
+body: |
+ bb.0:
+ successors: %bb.1, %bb.2
+ S_CBRANCH_VCCZ %bb.2, implicit $vcc
+ S_BRANCH %bb.1
+
+ bb.1:
+ successors: %bb.3
+ S_SETREG_IMM32_B32 3, 2177, implicit-def $mode, implicit $mode
+ S_BRANCH %bb.3
+
+ bb.2:
+ successors: %bb.3
+ liveins: $vgpr0_vgpr1, $vgpr2_vgpr3
+ $vgpr4_vgpr5 = V_ADD_F64_e64 0, $vgpr0_vgpr1, 0, $vgpr2_vgpr3, 0, 0, implicit $mode, implicit $exec
+ S_BRANCH %bb.3
+
+ bb.3:
+ liveins: $vgpr0_vgpr1, $vgpr2_vgpr3
+ $vgpr6_vgpr7 = V_ADD_F64_e64 0, $vgpr0_vgpr1, 0, $vgpr2_vgpr3, 0, 0, implicit $mode, implicit $exec
+ S_ENDPGM 0
+...
+---
+# A self-looping entry block used to be pinned to the default status without
+# folding in its own exit, dropping the RTN restore for the V_ADD_F64.
+# CHECK-LABEL: name: entry_self_loop_carried_mode
+# CHECK-LABEL: bb.0:
+# CHECK: S_SETREG_IMM32_B32 0, 2177
+# CHECK-NEXT: V_ADD_F64_e64
+# CHECK: S_SETREG_IMM32_B32 3, 2177
+# CHECK-NEXT: V_CVT_F32_F64_e32
+
+name: entry_self_loop_carried_mode
+
+body: |
+ bb.0:
+ successors: %bb.0, %bb.1
+ liveins: $vgpr0_vgpr1, $vgpr2_vgpr3
+ $vgpr4_vgpr5 = V_ADD_F64_e64 0, $vgpr0_vgpr1, 0, $vgpr2_vgpr3, 0, 0, implicit $mode, implicit $exec
+ $vgpr6 = FPTRUNC_ROUND_F32_F64_PSEUDO $vgpr4_vgpr5, 3, implicit $mode, implicit $exec
+ S_CBRANCH_VCCZ %bb.0, implicit $vcc
+ S_BRANCH %bb.1
+
+ bb.1:
+ S_ENDPGM 0
+...
``````````
</details>
https://github.com/llvm/llvm-project/pull/222244
More information about the llvm-commits
mailing list