[llvm] [AMDGPU] Do not treat bitcast across FP types as canonicality-preserving (PR #203560)
Arseniy Obolenskiy via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 31 05:47:34 PDT 2026
https://github.com/aobolensk updated https://github.com/llvm/llvm-project/pull/203560
>From bc20b0cee886844899b231949d459d6a4a870044 Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Fri, 12 Jun 2026 16:57:31 +0200
Subject: [PATCH 1/5] [AMDGPU] Do not treat bitcast across FP types as
canonicality-preserving
isCanonicalized recursed through ISD::BITCAST ignoring the type change, so value canonical as v2bf16 was wrongly treated as canonical when bitcast to v2f16 (that has different exponent width), dropping a required fcanonicalize
---
llvm/lib/Target/AMDGPU/SIISelLowering.cpp | 12 +++++---
.../AMDGPU/fcanonicalize-elimination.bf16.ll | 28 +++++++++++++++++++
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index eb3b869990cdd..1591a97bfc6db 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -15795,11 +15795,15 @@ bool SITargetLowering::isCanonicalized(SelectionDAG &DAG, SDValue Op,
// Could be anything.
return false;
- case ISD::BITCAST:
- // TODO: This is incorrect as it loses track of the operand's type. We may
- // end up effectively bitcasting from f32 to v2f16 or vice versa, and the
- // same bits that are canonicalized in one type need not be in the other.
+ case ISD::BITCAST: {
+ // Canonicality isn't preserved across a bitcast that changes the FP type.
+ EVT SrcVT = Op.getOperand(0).getValueType();
+ EVT DstVT = Op.getValueType();
+ if (SrcVT.isFloatingPoint() && DstVT.isFloatingPoint() &&
+ SrcVT.getScalarType() != DstVT.getScalarType())
+ return false;
return isCanonicalized(DAG, Op.getOperand(0), MaxDepth - 1);
+ }
case ISD::TRUNCATE: {
// Hack round the mess we make when legalizing extract_vector_elt
if (Op.getValueType() == MVT::i16) {
diff --git a/llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll b/llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll
index 0896a540d09d0..ee39b823778e6 100644
--- a/llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll
+++ b/llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll
@@ -63,3 +63,31 @@ define half @test_canonicalize_amdgcn_tanh_f16(half %a) {
%canonicalized = call half @llvm.canonicalize.f16(half %tanh)
ret half %canonicalized
}
+
+; A v2bf16 value bitcast to v2f16 may not be canonical as v2f16 (different
+; exponent width), so the v2f16 canonicalize must not be eliminated.
+define amdgpu_kernel void @test_no_eliminate_canonicalize_bitcast_bf16_to_f16(ptr addrspace(1) %in, ptr addrspace(1) %out) {
+; GCN-LABEL: test_no_eliminate_canonicalize_bitcast_bf16_to_f16:
+; GCN: ; %bb.0:
+; GCN-NEXT: s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 25, 1), 1 ; msbs: dst=0 src0=0 src1=0 src2=0
+; GCN-NEXT: s_load_b128 s[0:3], s[4:5], 0x0 nv
+; GCN-NEXT: v_mov_b32_e32 v1, 0
+; GCN-NEXT: s_wait_kmcnt 0x0
+; GCN-NEXT: s_load_b32 s0, s[0:1], 0x0
+; GCN-NEXT: s_wait_kmcnt 0x0
+; GCN-NEXT: v_pk_mul_bf16 v0, 1.0, s0 op_sel_hi:[0,1]
+; GCN-NEXT: s_delay_alu instid0(VALU_DEP_1)
+; GCN-NEXT: v_pk_max_num_f16 v0, v0, v0
+; GCN-NEXT: global_store_b32 v1, v0, s[2:3]
+; GCN-NEXT: s_endpgm
+ %x = load <2 x bfloat>, ptr addrspace(1) %in, align 4
+ %c = call <2 x bfloat> @llvm.canonicalize.v2bf16(<2 x bfloat> %x)
+ %h = bitcast <2 x bfloat> %c to <2 x half>
+ %lo = extractelement <2 x half> %h, i32 0
+ %hi = extractelement <2 x half> %h, i32 1
+ %v0 = insertelement <2 x half> poison, half %lo, i32 0
+ %v1 = insertelement <2 x half> %v0, half %hi, i32 1
+ %canon = call <2 x half> @llvm.canonicalize.v2f16(<2 x half> %v1)
+ store <2 x half> %canon, ptr addrspace(1) %out, align 4
+ ret void
+}
>From 6cac85d9766aeec9d7de30729b3a2bfadee527d7 Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Fri, 12 Jun 2026 20:41:02 +0200
Subject: [PATCH 2/5] peekThroughBitcasts
---
llvm/lib/Target/AMDGPU/SIISelLowering.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index 1591a97bfc6db..7032202b4f9b2 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -15797,12 +15797,13 @@ bool SITargetLowering::isCanonicalized(SelectionDAG &DAG, SDValue Op,
case ISD::BITCAST: {
// Canonicality isn't preserved across a bitcast that changes the FP type.
- EVT SrcVT = Op.getOperand(0).getValueType();
+ SDValue Src = peekThroughBitcasts(Op.getOperand(0));
+ EVT SrcVT = Src.getValueType();
EVT DstVT = Op.getValueType();
if (SrcVT.isFloatingPoint() && DstVT.isFloatingPoint() &&
SrcVT.getScalarType() != DstVT.getScalarType())
return false;
- return isCanonicalized(DAG, Op.getOperand(0), MaxDepth - 1);
+ return isCanonicalized(DAG, Src, MaxDepth - 1);
}
case ISD::TRUNCATE: {
// Hack round the mess we make when legalizing extract_vector_elt
>From 0b84925a0cf13ff0919c5f8928245808ca7a25be Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Tue, 30 Jun 2026 09:47:10 +0200
Subject: [PATCH 3/5] Address comments
---
llvm/lib/Target/AMDGPU/SIISelLowering.cpp | 55 ++++++++++++++++-------
llvm/lib/Target/AMDGPU/SIISelLowering.h | 2 +
2 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index 7032202b4f9b2..c6eb05dacd269 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -15645,6 +15645,21 @@ SDValue SITargetLowering::performRcpCombine(SDNode *N,
bool SITargetLowering::isCanonicalized(SelectionDAG &DAG, SDValue Op,
SDNodeFlags UserFlags,
unsigned MaxDepth) const {
+ EVT VT = Op.getValueType();
+ assert(VT.isFloatingPoint() &&
+ "expected a floating-point value to query canonicality of");
+ return isCanonicalized(DAG, Op, VT.getScalarType(), UserFlags, MaxDepth);
+}
+
+bool SITargetLowering::isCanonicalized(SelectionDAG &DAG, SDValue Op,
+ EVT QueryVT, SDNodeFlags UserFlags,
+ unsigned MaxDepth) const {
+ assert(QueryVT.isFloatingPoint() && !QueryVT.isVector() &&
+ "QueryVT must be a floating-point scalar type");
+ EVT VT = Op.getValueType();
+ if (VT.isFloatingPoint() && VT.getScalarType() != QueryVT)
+ return false;
+
unsigned Opcode = Op.getOpcode();
if (Opcode == ISD::FCANONICALIZE)
return true;
@@ -15713,7 +15728,8 @@ bool SITargetLowering::isCanonicalized(SelectionDAG &DAG, SDValue Op,
case ISD::FNEG:
case ISD::FABS:
case ISD::FCOPYSIGN:
- return isCanonicalized(DAG, Op.getOperand(0), MaxDepth - 1);
+ return isCanonicalized(DAG, Op.getOperand(0), QueryVT, UserFlags,
+ MaxDepth - 1);
case ISD::AND:
if (Op.getValueType() == MVT::i32) {
@@ -15723,7 +15739,8 @@ bool SITargetLowering::isCanonicalized(SelectionDAG &DAG, SDValue Op,
// is valid to optimize for all types.
if (auto *RHS = dyn_cast<ConstantSDNode>(Op.getOperand(1))) {
if (RHS->getZExtValue() == 0xffff0000) {
- return isCanonicalized(DAG, Op.getOperand(0), MaxDepth - 1);
+ return isCanonicalized(DAG, Op.getOperand(0), QueryVT, UserFlags,
+ MaxDepth - 1);
}
}
}
@@ -15764,20 +15781,23 @@ bool SITargetLowering::isCanonicalized(SelectionDAG &DAG, SDValue Op,
// FIXME: Does this apply with clamp? It's implemented with max.
for (unsigned I = 0, E = Op.getNumOperands(); I != E; ++I) {
- if (!isCanonicalized(DAG, Op.getOperand(I), MaxDepth - 1))
+ if (!isCanonicalized(DAG, Op.getOperand(I), QueryVT, UserFlags,
+ MaxDepth - 1))
return false;
}
return true;
}
case ISD::SELECT: {
- return isCanonicalized(DAG, Op.getOperand(1), MaxDepth - 1) &&
- isCanonicalized(DAG, Op.getOperand(2), MaxDepth - 1);
+ return isCanonicalized(DAG, Op.getOperand(1), QueryVT, UserFlags,
+ MaxDepth - 1) &&
+ isCanonicalized(DAG, Op.getOperand(2), QueryVT, UserFlags,
+ MaxDepth - 1);
}
case ISD::BUILD_VECTOR: {
for (unsigned i = 0, e = Op.getNumOperands(); i != e; ++i) {
SDValue SrcOp = Op.getOperand(i);
- if (!isCanonicalized(DAG, SrcOp, MaxDepth - 1))
+ if (!isCanonicalized(DAG, SrcOp, QueryVT, UserFlags, MaxDepth - 1))
return false;
}
@@ -15785,25 +15805,25 @@ bool SITargetLowering::isCanonicalized(SelectionDAG &DAG, SDValue Op,
}
case ISD::EXTRACT_VECTOR_ELT:
case ISD::EXTRACT_SUBVECTOR: {
- return isCanonicalized(DAG, Op.getOperand(0), MaxDepth - 1);
+ return isCanonicalized(DAG, Op.getOperand(0), QueryVT, UserFlags,
+ MaxDepth - 1);
}
case ISD::INSERT_VECTOR_ELT: {
- return isCanonicalized(DAG, Op.getOperand(0), MaxDepth - 1) &&
- isCanonicalized(DAG, Op.getOperand(1), MaxDepth - 1);
+ return isCanonicalized(DAG, Op.getOperand(0), QueryVT, UserFlags,
+ MaxDepth - 1) &&
+ isCanonicalized(DAG, Op.getOperand(1), QueryVT, UserFlags,
+ MaxDepth - 1);
}
case ISD::UNDEF:
// Could be anything.
return false;
case ISD::BITCAST: {
- // Canonicality isn't preserved across a bitcast that changes the FP type.
+ // Carry QueryVT through the bitcast unchanged. The top-of-function guard
+ // rejects a source whose FP format differs from the consumed type, so a
+ // value canonical in one FP format is not assumed canonical in another.
SDValue Src = peekThroughBitcasts(Op.getOperand(0));
- EVT SrcVT = Src.getValueType();
- EVT DstVT = Op.getValueType();
- if (SrcVT.isFloatingPoint() && DstVT.isFloatingPoint() &&
- SrcVT.getScalarType() != DstVT.getScalarType())
- return false;
- return isCanonicalized(DAG, Src, MaxDepth - 1);
+ return isCanonicalized(DAG, Src, QueryVT, UserFlags, MaxDepth - 1);
}
case ISD::TRUNCATE: {
// Hack round the mess we make when legalizing extract_vector_elt
@@ -15812,7 +15832,8 @@ bool SITargetLowering::isCanonicalized(SelectionDAG &DAG, SDValue Op,
if (TruncSrc.getValueType() == MVT::i32 &&
TruncSrc.getOpcode() == ISD::BITCAST &&
TruncSrc.getOperand(0).getValueType() == MVT::v2f16) {
- return isCanonicalized(DAG, TruncSrc.getOperand(0), MaxDepth - 1);
+ return isCanonicalized(DAG, TruncSrc.getOperand(0), QueryVT, UserFlags,
+ MaxDepth - 1);
}
}
return false;
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.h b/llvm/lib/Target/AMDGPU/SIISelLowering.h
index 95ff5bba7cfff..0f8476dfdb310 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.h
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.h
@@ -568,6 +568,8 @@ class SITargetLowering final : public AMDGPUTargetLowering {
bool isCanonicalized(SelectionDAG &DAG, SDValue Op,
SDNodeFlags UserFlags = {}, unsigned MaxDepth = 5) const;
+ bool isCanonicalized(SelectionDAG &DAG, SDValue Op, EVT QueryVT,
+ SDNodeFlags UserFlags, unsigned MaxDepth) const;
bool isCanonicalized(Register Reg, const MachineFunction &MF,
unsigned MaxDepth = 5) const;
bool denormalsEnabledForType(const SelectionDAG &DAG, EVT VT) const;
>From c4877d103324a568447c7ecd60c1f4670ffe1dc3 Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Fri, 31 Jul 2026 13:47:06 +0200
Subject: [PATCH 4/5] Address comments
---
llvm/lib/Target/AMDGPU/SIISelLowering.h | 7 +++++++
.../AMDGPU/fcanonicalize-elimination.bf16.ll | 19 +++++--------------
2 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.h b/llvm/lib/Target/AMDGPU/SIISelLowering.h
index 0f8476dfdb310..74e9f63bd493a 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.h
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.h
@@ -568,6 +568,13 @@ class SITargetLowering final : public AMDGPUTargetLowering {
bool isCanonicalized(SelectionDAG &DAG, SDValue Op,
SDNodeFlags UserFlags = {}, unsigned MaxDepth = 5) const;
+
+ /// Returns true if \p Op is provably canonical (no FCANONICALIZE needed).
+ /// \p QueryVT is the scalar FP type being checked, threaded unchanged
+ /// through recursion since canonicality is per vector element. FP operands
+ /// whose scalar type differs from \p QueryVT are treated as non-canonical,
+ /// since canonicality does not survive a change of FP format (e.g. bitcast
+ /// v2bf16 to v2f16); non-FP operands are not checked against \p QueryVT.
bool isCanonicalized(SelectionDAG &DAG, SDValue Op, EVT QueryVT,
SDNodeFlags UserFlags, unsigned MaxDepth) const;
bool isCanonicalized(Register Reg, const MachineFunction &MF,
diff --git a/llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll b/llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll
index ee39b823778e6..ee12a5140e252 100644
--- a/llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll
+++ b/llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll
@@ -66,21 +66,13 @@ define half @test_canonicalize_amdgcn_tanh_f16(half %a) {
; A v2bf16 value bitcast to v2f16 may not be canonical as v2f16 (different
; exponent width), so the v2f16 canonicalize must not be eliminated.
-define amdgpu_kernel void @test_no_eliminate_canonicalize_bitcast_bf16_to_f16(ptr addrspace(1) %in, ptr addrspace(1) %out) {
+define <2 x half> @test_no_eliminate_canonicalize_bitcast_bf16_to_f16(<2 x bfloat> %x) {
; GCN-LABEL: test_no_eliminate_canonicalize_bitcast_bf16_to_f16:
; GCN: ; %bb.0:
-; GCN-NEXT: s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 25, 1), 1 ; msbs: dst=0 src0=0 src1=0 src2=0
-; GCN-NEXT: s_load_b128 s[0:3], s[4:5], 0x0 nv
-; GCN-NEXT: v_mov_b32_e32 v1, 0
-; GCN-NEXT: s_wait_kmcnt 0x0
-; GCN-NEXT: s_load_b32 s0, s[0:1], 0x0
+; GCN-NEXT: s_wait_loadcnt_dscnt 0x0
; GCN-NEXT: s_wait_kmcnt 0x0
-; GCN-NEXT: v_pk_mul_bf16 v0, 1.0, s0 op_sel_hi:[0,1]
-; GCN-NEXT: s_delay_alu instid0(VALU_DEP_1)
-; GCN-NEXT: v_pk_max_num_f16 v0, v0, v0
-; GCN-NEXT: global_store_b32 v1, v0, s[2:3]
-; GCN-NEXT: s_endpgm
- %x = load <2 x bfloat>, ptr addrspace(1) %in, align 4
+; GCN-NEXT: v_pk_mul_bf16 v0, 1.0, v0 op_sel_hi:[0,1]
+; GCN-NEXT: s_set_pc_i64 s[30:31]
%c = call <2 x bfloat> @llvm.canonicalize.v2bf16(<2 x bfloat> %x)
%h = bitcast <2 x bfloat> %c to <2 x half>
%lo = extractelement <2 x half> %h, i32 0
@@ -88,6 +80,5 @@ define amdgpu_kernel void @test_no_eliminate_canonicalize_bitcast_bf16_to_f16(pt
%v0 = insertelement <2 x half> poison, half %lo, i32 0
%v1 = insertelement <2 x half> %v0, half %hi, i32 1
%canon = call <2 x half> @llvm.canonicalize.v2f16(<2 x half> %v1)
- store <2 x half> %canon, ptr addrspace(1) %out, align 4
- ret void
+ ret <2 x half> %canon
}
>From 4938eadbc72675cfdb15b8b9e0a58cd3e8377c3f Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Fri, 31 Jul 2026 14:19:13 +0200
Subject: [PATCH 5/5] fix test
---
llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll | 2 ++
1 file changed, 2 insertions(+)
diff --git a/llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll b/llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll
index ee12a5140e252..858faa1959dd9 100644
--- a/llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll
+++ b/llvm/test/CodeGen/AMDGPU/fcanonicalize-elimination.bf16.ll
@@ -72,6 +72,8 @@ define <2 x half> @test_no_eliminate_canonicalize_bitcast_bf16_to_f16(<2 x bfloa
; GCN-NEXT: s_wait_loadcnt_dscnt 0x0
; GCN-NEXT: s_wait_kmcnt 0x0
; GCN-NEXT: v_pk_mul_bf16 v0, 1.0, v0 op_sel_hi:[0,1]
+; GCN-NEXT: s_delay_alu instid0(VALU_DEP_1)
+; GCN-NEXT: v_pk_max_num_f16 v0, v0, v0
; GCN-NEXT: s_set_pc_i64 s[30:31]
%c = call <2 x bfloat> @llvm.canonicalize.v2bf16(<2 x bfloat> %x)
%h = bitcast <2 x bfloat> %c to <2 x half>
More information about the llvm-commits
mailing list