[llvm] 807de2d - [AMDGPU] Stop iDot4 chain walker at non-ADD nodes (#198412)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 28 08:01:55 PDT 2026
Author: Arseniy Obolenskiy
Date: 2026-07-28T17:01:50+02:00
New Revision: 807de2dc44794205b336ed8b2ca4731bc3ab865b
URL: https://github.com/llvm/llvm-project/commit/807de2dc44794205b336ed8b2ca4731bc3ab865b
DIFF: https://github.com/llvm/llvm-project/commit/807de2dc44794205b336ed8b2ca4731bc3ab865b.diff
LOG: [AMDGPU] Stop iDot4 chain walker at non-ADD nodes (#198412)
The loop body in performAddCombine dot4 matcher unconditionally treats
TempNode operands as the next link's addends, so the chain only works
when TempNode is an `ISD::ADD`
The old getNumOperands() guard let AND/OR/XOR/etc. through, leaking
their non-addend operands (e.g. a constant mask) into the dot4
accumulator and miscompiling kernels
Added:
Modified:
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
llvm/test/CodeGen/AMDGPU/idot4-test.ll
llvm/test/CodeGen/AMDGPU/no-corresponding-integer-type.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index f2b670558de1c..78bbb7f2d6146 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -17567,7 +17567,8 @@ SDValue SITargetLowering::performAddCombine(SDNode *N,
TempNode = TempNode->getOperand(AddIdx);
Src2s.push_back(TempNode);
ChainLength = I + 1;
- if (TempNode->getNumOperands() < 2)
+ // The loop body treats TempNode's operands as addends.
+ if (TempNode.getOpcode() != ISD::ADD)
break;
LHS = TempNode->getOperand(0);
RHS = TempNode->getOperand(1);
diff --git a/llvm/test/CodeGen/AMDGPU/idot4-test.ll b/llvm/test/CodeGen/AMDGPU/idot4-test.ll
index 7a11389b0393e..d055059751205 100644
--- a/llvm/test/CodeGen/AMDGPU/idot4-test.ll
+++ b/llvm/test/CodeGen/AMDGPU/idot4-test.ll
@@ -939,6 +939,233 @@ entry:
ret i32 %result
}
+;------------------------------------------------------------------------------
+; NEGATIVE TESTS: CHAIN CONTAINS A NON-ADD NODE
+;------------------------------------------------------------------------------
+
+define i32 @dot4_and_chain_not_add(i32 %v) {
+; GFX9-DL-LABEL: dot4_and_chain_not_add:
+; GFX9-DL: ; %bb.0:
+; GFX9-DL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX9-DL-NEXT: v_mul_u32_u24_sdwa v0, v0, v0 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX9-DL-NEXT: v_and_b32_e32 v1, 1, v0
+; GFX9-DL-NEXT: v_add_u32_e32 v0, v1, v0
+; GFX9-DL-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX10-DL-LABEL: dot4_and_chain_not_add:
+; GFX10-DL: ; %bb.0:
+; GFX10-DL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX10-DL-NEXT: v_mul_u32_u24_sdwa v0, v0, v0 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX10-DL-NEXT: v_and_b32_e32 v1, 1, v0
+; GFX10-DL-NEXT: v_add_nc_u32_e32 v0, v1, v0
+; GFX10-DL-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX950-LABEL: dot4_and_chain_not_add:
+; GFX950: ; %bb.0:
+; GFX950-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX950-NEXT: v_mul_u32_u24_sdwa v0, v0, v0 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX950-NEXT: v_and_b32_e32 v1, 1, v0
+; GFX950-NEXT: v_add_u32_e32 v0, v1, v0
+; GFX950-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX9-NODL-LABEL: dot4_and_chain_not_add:
+; GFX9-NODL: ; %bb.0:
+; GFX9-NODL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX9-NODL-NEXT: v_mul_u32_u24_sdwa v0, v0, v0 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX9-NODL-NEXT: v_and_b32_e32 v1, 1, v0
+; GFX9-NODL-NEXT: v_add_u32_e32 v0, v1, v0
+; GFX9-NODL-NEXT: s_setpc_b64 s[30:31]
+ %masked = and i32 %v, 255
+ %square = mul i32 %masked, %masked
+ %lowbit = and i32 %square, 1
+ %result = add i32 %lowbit, %square
+ ret i32 %result
+}
+
+define i32 @dot4_or_chain_not_add(i32 %x, i32 %y, i32 %z) {
+; GFX9-DL-LABEL: dot4_or_chain_not_add:
+; GFX9-DL: ; %bb.0:
+; GFX9-DL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX9-DL-NEXT: v_mul_u32_u24_sdwa v1, v0, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX9-DL-NEXT: v_or_b32_e32 v1, 1, v1
+; GFX9-DL-NEXT: v_mul_u32_u24_sdwa v0, v0, v2 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX9-DL-NEXT: v_add_u32_e32 v0, v0, v1
+; GFX9-DL-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX10-DL-LABEL: dot4_or_chain_not_add:
+; GFX10-DL: ; %bb.0:
+; GFX10-DL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX10-DL-NEXT: v_mul_u32_u24_sdwa v1, v0, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX10-DL-NEXT: v_mul_u32_u24_sdwa v0, v0, v2 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX10-DL-NEXT: v_or_b32_e32 v1, 1, v1
+; GFX10-DL-NEXT: v_add_nc_u32_e32 v0, v0, v1
+; GFX10-DL-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX950-LABEL: dot4_or_chain_not_add:
+; GFX950: ; %bb.0:
+; GFX950-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX950-NEXT: v_mul_u32_u24_sdwa v1, v0, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX950-NEXT: v_or_b32_e32 v1, 1, v1
+; GFX950-NEXT: v_mul_u32_u24_sdwa v0, v0, v2 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX950-NEXT: v_add_u32_e32 v0, v0, v1
+; GFX950-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX9-NODL-LABEL: dot4_or_chain_not_add:
+; GFX9-NODL: ; %bb.0:
+; GFX9-NODL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX9-NODL-NEXT: v_mul_u32_u24_sdwa v1, v0, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX9-NODL-NEXT: v_or_b32_e32 v1, 1, v1
+; GFX9-NODL-NEXT: v_mul_u32_u24_sdwa v0, v0, v2 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX9-NODL-NEXT: v_add_u32_e32 v0, v0, v1
+; GFX9-NODL-NEXT: s_setpc_b64 s[30:31]
+ %x.m = and i32 %x, 255
+ %y.m = and i32 %y, 255
+ %z.m = and i32 %z, 255
+ %mul0 = mul i32 %x.m, %y.m
+ %or = or i32 %mul0, 1
+ %mul1 = mul i32 %x.m, %z.m
+ %result = add i32 %mul1, %or
+ ret i32 %result
+}
+
+; `or disjoint x, c` is equivalent to `add x, c`; this currently is not folded
+; into the dot4 either because the matcher only accepts ISD::ADD. If that
+; restriction is loosened, this becomes the positive case.
+define i32 @dot4_or_disjoint_chain_not_add(i32 %x, i32 %y, i32 %z) {
+; GFX9-DL-LABEL: dot4_or_disjoint_chain_not_add:
+; GFX9-DL: ; %bb.0:
+; GFX9-DL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX9-DL-NEXT: v_and_b32_e32 v0, 0xfe, v0
+; GFX9-DL-NEXT: v_mul_u32_u24_sdwa v1, v0, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:BYTE_0
+; GFX9-DL-NEXT: v_mul_u32_u24_sdwa v0, v0, v2 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:BYTE_0
+; GFX9-DL-NEXT: v_add_u32_e32 v0, v0, v1
+; GFX9-DL-NEXT: v_or_b32_e32 v0, 1, v0
+; GFX9-DL-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX10-DL-LABEL: dot4_or_disjoint_chain_not_add:
+; GFX10-DL: ; %bb.0:
+; GFX10-DL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX10-DL-NEXT: v_and_b32_e32 v0, 0xfe, v0
+; GFX10-DL-NEXT: v_mul_u32_u24_sdwa v1, v0, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:BYTE_0
+; GFX10-DL-NEXT: v_mul_u32_u24_sdwa v0, v0, v2 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:BYTE_0
+; GFX10-DL-NEXT: v_add_nc_u32_e32 v0, v0, v1
+; GFX10-DL-NEXT: v_or_b32_e32 v0, 1, v0
+; GFX10-DL-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX950-LABEL: dot4_or_disjoint_chain_not_add:
+; GFX950: ; %bb.0:
+; GFX950-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX950-NEXT: v_and_b32_e32 v0, 0xfe, v0
+; GFX950-NEXT: v_mul_u32_u24_sdwa v1, v0, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:BYTE_0
+; GFX950-NEXT: v_mul_u32_u24_sdwa v0, v0, v2 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:BYTE_0
+; GFX950-NEXT: v_add_u32_e32 v0, v0, v1
+; GFX950-NEXT: v_or_b32_e32 v0, 1, v0
+; GFX950-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX9-NODL-LABEL: dot4_or_disjoint_chain_not_add:
+; GFX9-NODL: ; %bb.0:
+; GFX9-NODL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX9-NODL-NEXT: v_and_b32_e32 v0, 0xfe, v0
+; GFX9-NODL-NEXT: v_mul_u32_u24_sdwa v1, v0, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:BYTE_0
+; GFX9-NODL-NEXT: v_mul_u32_u24_sdwa v0, v0, v2 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:BYTE_0
+; GFX9-NODL-NEXT: v_add_u32_e32 v0, v0, v1
+; GFX9-NODL-NEXT: v_or_b32_e32 v0, 1, v0
+; GFX9-NODL-NEXT: s_setpc_b64 s[30:31]
+ %x.m = and i32 %x, 254
+ %y.m = and i32 %y, 255
+ %z.m = and i32 %z, 255
+ %mul0 = mul i32 %x.m, %y.m
+ %or = or disjoint i32 %mul0, 1
+ %mul1 = mul i32 %x.m, %z.m
+ %result = add i32 %mul1, %or
+ ret i32 %result
+}
+
+; Same shape as the udot4 case, but signed. The matcher uses a shared loop, so
+; the signed path must not regress either.
+define i32 @sdot4_and_chain_not_add(i32 %v) {
+; GFX9-DL-LABEL: sdot4_and_chain_not_add:
+; GFX9-DL: ; %bb.0:
+; GFX9-DL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX9-DL-NEXT: v_mul_i32_i24_sdwa v0, sext(v0), sext(v0) dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX9-DL-NEXT: v_and_b32_e32 v1, 1, v0
+; GFX9-DL-NEXT: v_add_u32_e32 v0, v1, v0
+; GFX9-DL-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX10-DL-LABEL: sdot4_and_chain_not_add:
+; GFX10-DL: ; %bb.0:
+; GFX10-DL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX10-DL-NEXT: v_mul_i32_i24_sdwa v0, sext(v0), sext(v0) dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX10-DL-NEXT: v_and_b32_e32 v1, 1, v0
+; GFX10-DL-NEXT: v_add_nc_u32_e32 v0, v1, v0
+; GFX10-DL-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX950-LABEL: sdot4_and_chain_not_add:
+; GFX950: ; %bb.0:
+; GFX950-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX950-NEXT: v_mul_i32_i24_sdwa v0, sext(v0), sext(v0) dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX950-NEXT: v_and_b32_e32 v1, 1, v0
+; GFX950-NEXT: v_add_u32_e32 v0, v1, v0
+; GFX950-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX9-NODL-LABEL: sdot4_and_chain_not_add:
+; GFX9-NODL: ; %bb.0:
+; GFX9-NODL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX9-NODL-NEXT: v_mul_i32_i24_sdwa v0, sext(v0), sext(v0) dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX9-NODL-NEXT: v_and_b32_e32 v1, 1, v0
+; GFX9-NODL-NEXT: v_add_u32_e32 v0, v1, v0
+; GFX9-NODL-NEXT: s_setpc_b64 s[30:31]
+ %shl = shl i32 %v, 24
+ %byte = ashr i32 %shl, 24
+ %square = mul i32 %byte, %byte
+ %lowbit = and i32 %square, 1
+ %result = add i32 %lowbit, %square
+ ret i32 %result
+}
+
+; Before the chain-shape guard the addend walker stepped into the AND
+; and pushed its 1023 mask as a dot4 accumulator, miscompiling the kernel
+; to 0x3ff for an all-zero input.
+define i32 @dot4_and_mask10_chain_not_add(i32 %x, i32 %y) {
+; GFX9-DL-LABEL: dot4_and_mask10_chain_not_add:
+; GFX9-DL: ; %bb.0:
+; GFX9-DL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX9-DL-NEXT: v_mul_u32_u24_sdwa v0, v0, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX9-DL-NEXT: v_and_b32_e32 v1, 0x3ff, v0
+; GFX9-DL-NEXT: v_add_u32_e32 v0, v1, v0
+; GFX9-DL-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX10-DL-LABEL: dot4_and_mask10_chain_not_add:
+; GFX10-DL: ; %bb.0:
+; GFX10-DL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX10-DL-NEXT: v_mul_u32_u24_sdwa v0, v0, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX10-DL-NEXT: v_and_b32_e32 v1, 0x3ff, v0
+; GFX10-DL-NEXT: v_add_nc_u32_e32 v0, v1, v0
+; GFX10-DL-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX950-LABEL: dot4_and_mask10_chain_not_add:
+; GFX950: ; %bb.0:
+; GFX950-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX950-NEXT: v_mul_u32_u24_sdwa v0, v0, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX950-NEXT: v_and_b32_e32 v1, 0x3ff, v0
+; GFX950-NEXT: v_add_u32_e32 v0, v1, v0
+; GFX950-NEXT: s_setpc_b64 s[30:31]
+;
+; GFX9-NODL-LABEL: dot4_and_mask10_chain_not_add:
+; GFX9-NODL: ; %bb.0:
+; GFX9-NODL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GFX9-NODL-NEXT: v_mul_u32_u24_sdwa v0, v0, v1 dst_sel:DWORD dst_unused:UNUSED_PAD src0_sel:BYTE_0 src1_sel:BYTE_0
+; GFX9-NODL-NEXT: v_and_b32_e32 v1, 0x3ff, v0
+; GFX9-NODL-NEXT: v_add_u32_e32 v0, v1, v0
+; GFX9-NODL-NEXT: s_setpc_b64 s[30:31]
+ %xb = and i32 %x, 255
+ %yb = and i32 %y, 255
+ %mul = mul i32 %xb, %yb
+ %masked = and i32 %mul, 1023
+ %result = add i32 %masked, %mul
+ ret i32 %result
+}
+
declare i32 @llvm.sadd.sat.i32(i32, i32)
declare i32 @llvm.uadd.sat.i32(i32, i32)
declare i32 @llvm.vector.reduce.add.v4i32(<4 x i32>)
diff --git a/llvm/test/CodeGen/AMDGPU/no-corresponding-integer-type.ll b/llvm/test/CodeGen/AMDGPU/no-corresponding-integer-type.ll
index aced83a9016cb..0d360942f006b 100644
--- a/llvm/test/CodeGen/AMDGPU/no-corresponding-integer-type.ll
+++ b/llvm/test/CodeGen/AMDGPU/no-corresponding-integer-type.ll
@@ -9,15 +9,10 @@ define void @no_corresponding_integer_type(i8 %arg, ptr addrspace(1) %ptr) {
; CHECK-NEXT: v_mov_b32_e32 v3, v2
; CHECK-NEXT: v_mov_b32_e32 v2, v1
; CHECK-NEXT: global_load_ushort v1, v[2:3], off
-; CHECK-NEXT: global_load_ubyte v4, v[2:3], off offset:2
-; CHECK-NEXT: s_mov_b32 s0, 0xc0c0400
-; CHECK-NEXT: s_mov_b32 s1, 0xc0c0000
; CHECK-NEXT: s_waitcnt vmcnt(0)
-; CHECK-NEXT: v_lshl_or_b32 v1, v4, 16, v1
-; CHECK-NEXT: v_perm_b32 v1, v0, v1, s0
-; CHECK-NEXT: v_perm_b32 v0, v0, v0, s1
-; CHECK-NEXT: v_dot4_u32_u8 v0, v0, v1, 1
-; CHECK-NEXT: s_nop 2
+; CHECK-NEXT: v_mul_lo_u16_e32 v1, v1, v0
+; CHECK-NEXT: v_or_b32_e32 v1, 1, v1
+; CHECK-NEXT: v_mad_legacy_u16 v0, v0, v0, v1
; CHECK-NEXT: global_store_byte v[2:3], v0, off
; CHECK-NEXT: s_waitcnt vmcnt(0)
; CHECK-NEXT: s_setpc_b64 s[30:31]
More information about the llvm-commits
mailing list