[PATCH] D104793: [ARM] Transform a floating-point to fixed-point conversion to a VCVT_fix

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:3180
+    return false;
+  if (ImmNode.getOpcode() == ISD::BITCAST)
+    ImmNode = ImmNode.getOperand(0);
----------------
Should this still check for the same size of the bitcast on ImmNode?


================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:3291
+
+  if (Node->getOpcode() != ISD::FMUL) {
+    return false;
----------------
This can drop the brackets, from a single statement.


================
Comment at: llvm/test/CodeGen/Thumb2/mve-vcvt-float-to-fixed.ll:311
+; CHECK-NEXT:    bx lr
+  %2 = fmul fast <4 x float> %0, <float 0xC1E0000000000000, float 0xC1E0000000000000, float 0xC1E0000000000000, float 0xC1E0000000000000>
+  %3 = fptosi <4 x float> %2 to <4 x i32>
----------------
Should this be 0x41E0000000000000 ?


================
Comment at: llvm/test/CodeGen/Thumb2/mve-vcvt-float-to-fixed.ll:323
+; CHECK-NEXT:    bx lr
+  %2 = fmul <4 x float> %0, <float 0x3DF0000000000000, float 0x3DF0000000000000, float 0x3DF0000000000000, float 0x3DF0000000000000>
+  %3 = fptosi <4 x float> %2 to <4 x i32>
----------------
And this 0x41F0000000000000


================
Comment at: llvm/test/CodeGen/Thumb2/mve-vcvt-float-to-fixed.ll:335
+; CHECK-NEXT:    bx lr
+  %2 = fmul <4 x float> %0, <float 0x3DE0000000000000, float 0x3DE0000000000000, float 0x3DE0000000000000, float 0x3DE0000000000000>
+  %3 = fptosi <4 x float> %2 to <4 x i32>
----------------
And this 0x4200000000000000 ?


================
Comment at: llvm/test/CodeGen/Thumb2/mve-vcvt-float-to-fixed.ll:1019
+; CHECK-NEXT:    bx lr
+  %2 = fmul <4 x float> %0, <float 0xBF00000000000000, float 0xBF00000000000000, float 0xBF00000000000000, float 0xBF00000000000000>
+  %3 = fptosi <4 x float> %2 to <4 x i32>
----------------
I'm not sure if these are interesting negative constants any more.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104793/new/

https://reviews.llvm.org/D104793