[AArch64] missed fold vector fptoui(fdiv(uitofp(x),uitofp(y))) or fptosi(fdiv(sitofp(x),sitofp(y))) into udiv(x,y) or sdiv(x,y) for sve - #215146
Conversation
|
@llvm/pr-subscribers-backend-aarch64 Author: Durgesh Nandan Mohanty (dnmohanty) ChangesThis PR implements a DAG combine in DAG Combine Logic: Added
Fixes #214927 Full diff: https://github.com/llvm/llvm-project/pull/215146.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 2ac6c5fbc471a..2bde72a6a9593 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -31167,7 +31167,36 @@ static SDValue performPredicateLoadCombine(SDNode *N,
DAG.makeEquivalentMemoryOrdering(Load, LoadPred);
return LoadPred;
}
+static SDValue performFPToIntToDivCombine(SDNode *N, SelectionDAG &DAG) {
+ unsigned Opc = N->getOpcode();
+ bool IsSigned = (Opc == ISD::FP_TO_SINT);
+
+ SDValue FDiv = N->getOperand(0);
+ if (FDiv.getOpcode() != ISD::FDIV)
+ return SDValue();
+
+ EVT IntVT = N->getValueType(0);
+ EVT FPVT = FDiv.getValueType();
+
+ if (IntVT.getVectorElementType() != MVT::i32 ||
+ FPVT.getVectorElementType() != MVT::f64)
+ return SDValue();
+
+ unsigned CastOpc = IsSigned ? ISD::SINT_TO_FP : ISD::UINT_TO_FP;
+ SDValue Op0 = FDiv.getOperand(0);
+ SDValue Op1 = FDiv.getOperand(1);
+ if (Op0.getOpcode() != CastOpc || Op1.getOpcode() != CastOpc)
+ return SDValue();
+
+ if (Op0.getOperand(0).getValueType() != IntVT ||
+ Op1.getOperand(0).getValueType() != IntVT)
+ return SDValue();
+
+ unsigned DivOpc = IsSigned ? ISD::SDIV : ISD::UDIV;
+ return DAG.getNode(DivOpc, SDLoc(N), IntVT, Op0.getOperand(0),
+ Op1.getOperand(0));
+}
SDValue AArch64TargetLowering::PerformDAGCombine(SDNode *N,
DAGCombinerInfo &DCI) const {
SelectionDAG &DAG = DCI.DAG;
@@ -31175,6 +31204,9 @@ SDValue AArch64TargetLowering::PerformDAGCombine(SDNode *N,
default:
LLVM_DEBUG(dbgs() << "Custom combining: skipping\n");
break;
+ case ISD::FP_TO_UINT:
+ case ISD::FP_TO_SINT:
+ return performFPToIntToDivCombine(N, DAG);
case ISD::VECTOR_DEINTERLEAVE:
return performVectorDeinterleaveCombine(N, DCI, DAG);
case ISD::VECREDUCE_AND:
diff --git a/llvm/test/CodeGen/AArch64/sve-fdiv-int-fold.ll b/llvm/test/CodeGen/AArch64/sve-fdiv-int-fold.ll
new file mode 100644
index 0000000000000..953187c7208d5
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/sve-fdiv-int-fold.ll
@@ -0,0 +1,34 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+
+define <vscale x 2 x i32> @fptoui_fdiv_uitofp_nxv2i32(<vscale x 2 x i32> %a, <vscale x 2 x i32> %b) {
+; CHECK-LABEL: fptoui_fdiv_uitofp_nxv2i32:
+; CHECK: // %bb.0:
+; CHECK-NEXT: ptrue p0.d
+; CHECK-NEXT: ucvtf z0.d, p0/m, z0.s
+; CHECK-NEXT: ucvtf z1.d, p0/m, z1.s
+; CHECK-NEXT: fdiv z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: fcvtzs z0.d, p0/m, z0.d
+; CHECK-NEXT: ret
+ %fa = uitofp <vscale x 2 x i32> %a to <vscale x 2 x double>
+ %fb = uitofp <vscale x 2 x i32> %b to <vscale x 2 x double>
+ %fdiv = fdiv <vscale x 2 x double> %fa, %fb
+ %res = fptoui <vscale x 2 x double> %fdiv to <vscale x 2 x i32>
+ ret <vscale x 2 x i32> %res
+}
+
+define <vscale x 2 x i32> @fptosi_fdiv_sitofp_nxv2i32(<vscale x 2 x i32> %a, <vscale x 2 x i32> %b) {
+; CHECK-LABEL: fptosi_fdiv_sitofp_nxv2i32:
+; CHECK: // %bb.0:
+; CHECK-NEXT: ptrue p0.d
+; CHECK-NEXT: scvtf z0.d, p0/m, z0.s
+; CHECK-NEXT: scvtf z1.d, p0/m, z1.s
+; CHECK-NEXT: fdiv z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: fcvtzs z0.d, p0/m, z0.d
+; CHECK-NEXT: ret
+ %fa = sitofp <vscale x 2 x i32> %a to <vscale x 2 x double>
+ %fb = sitofp <vscale x 2 x i32> %b to <vscale x 2 x double>
+ %fdiv = fdiv <vscale x 2 x double> %fa, %fb
+ %res = fptosi <vscale x 2 x double> %fdiv to <vscale x 2 x i32>
+ ret <vscale x 2 x i32> %res
+}
|
| EVT IntVT = N->getValueType(0); | ||
| EVT FPVT = FDiv.getValueType(); | ||
|
|
||
| if (IntVT.getVectorElementType() != MVT::i32 || |
There was a problem hiding this comment.
That assumes that the type is a vector. Why is that important anyway?
There was a problem hiding this comment.
I've added the isVector() safety check in the latest commit. It's important because the original issue specifically targets SVE vector folding, and as Andarwinux noted, we need to strictly restrict this fold to i32-f64-i32 vector conversions to avoid altering the original behavior for other types
| return SDValue(); | ||
|
|
||
| unsigned DivOpc = IsSigned ? ISD::SDIV : ISD::UDIV; | ||
| return DAG.getNode(DivOpc, SDLoc(N), IntVT, Op0.getOperand(0), |
There was a problem hiding this comment.
I think this transformation is problematic whenever the divisor is zero which would cause UB.
There was a problem hiding this comment.
If a signed integer division (INT_MIN / -1) is performed where INT_MIN is the most negative integer value representable in the selected register size, then the result overflows the signed integer range. No indication of this overflow is produced and the result that is written to the destination register is INT_MIN.
A division by zero results in a zero being written to the destination register, without any indication that the division by zero occurred.
https://llvm.org/docs/LangRef.html#fptoui-to-instruction
The ‘fptoui’ instruction converts its floating-point operand into the nearest (rounding towards zero) unsigned integer value. If the value cannot fit in ty2, the result is a poison value.
The ‘fptosi’ instruction converts its floating-point operand into the nearest (rounding towards zero) signed integer value. If the value cannot fit in ty2, the result is a poison value.
It's still just return a poison.
135937d to
7b9c170
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
06605f9 to
c45c77f
Compare
c45c77f to
4f3696c
Compare
🪟 Windows x64 Test ResultsThe build failed before running any tests. Click on a failure below to see the details. [code=1] lib/Target/AArch64/CMakeFiles/LLVMAArch64CodeGen.dir/AArch64ISelLowering.cpp.objIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
🐧 Linux x64 Test ResultsThe build failed before running any tests. Click on a failure below to see the details. lib/Target/AArch64/CMakeFiles/LLVMAArch64CodeGen.dir/AArch64ISelLowering.cpp.oIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
|
@guy-david / @Andarwinux, I noticed the CI builds are failing due to some missing AArch64ISD opcodes (like SADDWB). Since my code doesn't interact with those lines, I'm wondering if I accidentally pulled in a temporary broken state from main, or if I genuinely messed something up on my end? let me know if I need to fix anything |
This PR implements a DAG combine in
AArch64ISelLowering.cppto fold vector floating-point division and conversions into native SVE integer division (udiv/sdiv).DAG Combine Logic: Added
performFPToIntToDivCombinesupportingISD::FP_TO_UINTand ISD::FP_TO_SINTwith strict type checks ensuring safe conversions (i32vectors viaf64). Tests: Addedllvm/test/CodeGen/AArch64/sve-fdiv-int-fold.ll` to verify optimal single-instruction codegen.Commit History: Structured into two clean commits as requested:
Fixes #214927