[PATCH] D104793: [ARM] Transform a floating-point to fixed-point conversion to a VCVT_fix
Dave Green via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 24 05:50:18 PDT 2021
dmgreen added a comment.
Nice. I wasn't expecting this to share as much code, that's good to see.
================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:3157
-bool ARMDAGToDAGISel::tryFMULFixed(SDNode *N, SDLoc dl) {
- // Transform a fixed-point to floating-point conversion to a VCVT
- if (!Subtarget->hasMVEFloatOps())
- return false;
+bool ARMDAGToDAGISel::transformFixedFloatingPointConversion(SDNode *N, SDLoc dl,
+ SDNode *FMul,
----------------
You can likely not pass dl, using SDLoc(N) again here.
================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:3173-3174
+ auto ImmNode = FMul->getOperand(1);
+ auto VecVal = FMul->getOperand(0);
+
----------------
auto -> SDValue
Should VecVal be different for the two cases? Otherwise below it uses the fmul->operand(0)->operand(0).
================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:3176
+
+ if (VecVal.getValueType().getVectorElementType().getSizeInBits() !=
+ ScalarBits)
----------------
Can this use getScalarSizeInBits?
================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:3204
- // Multiplying by a factor of 2^(-n) will convert from fixed point to
- // floating point, where n is the number of fractional bits in the fixed
- // point number. Taking the inverse and log2 of the factor will give n
- APFloat Inverse(0.0f);
- if (!ImmAPF.getExactInverse(&Inverse))
- return false;
-
+ // Multiplying by a factor of 2^n will convert from floating point to
+ // fixed point, where n is the number of fractional bits in the fixed
----------------
Maybe expand the comment to talk about both FixedToFloat and FloatToFixed
================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:3254
+ return false;
+ auto Type = N->getValueType(0);
+ if (!Type.isVector())
----------------
auto -> EVT
================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:3260
+ bool IsUnsigned = N->getOpcode() == ISD::FP_TO_UINT;
+ auto Node = N->getOperand(0).getNode();
+
----------------
auto -> SDNode *
================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:3310
+ return transformFixedFloatingPointConversion(
+ N, dl, N, LHS.getOpcode() == ISD::UINT_TO_FP, true);
+}
----------------
The second N should be LHS?
================
Comment at: llvm/test/CodeGen/Thumb2/mve-vcvt-float-to-fixed.ll:802
+
+define arm_aapcs_vfpcc <4 x float> @vcvt_u32_32(<4 x i32> %0) {
+; CHECK-LABEL: vcvt_u32_32:
----------------
Does this need updating? and vcvt_u32_33 below.
================
Comment at: llvm/test/CodeGen/Thumb2/mve-vcvt-float-to-fixed.ll:986
+
+define arm_aapcs_vfpcc <8 x i16> @vcvt_s16_inf(<8 x half> %0) {
+; CHECK-LABEL: vcvt_s16_inf:
----------------
These too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104793/new/
https://reviews.llvm.org/D104793
More information about the llvm-commits
mailing list