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

[Sam Tebbs via Phabricator via llvm-commits]

samtebbs added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:199
   bool tryMVEIndexedLoad(SDNode *N);
+  bool tryFMUL(SDNode *N, SDLoc dl);
 
----------------
dmgreen wrote:
> Perhaps tryFMulFixed, to explain a little more what the function tries to match?
Sounds good to me


================
Comment at: llvm/lib/Target/ARM/ARMISelDAGToDAG.cpp:3192
+    // that don't
+    if (!dyn_cast<ConstantSDNode>(RHS.getOperand(0)))
+      return false;
----------------
dmgreen wrote:
> Can use isa, as opposed to dyn_cast.
> 
> I'm not sure what the TODO is related to?
Good idea 👍 

The TODO is saying that when we find examples of VDUPs that don't have a constant operand we should modify this to work with them.


================
Comment at: llvm/test/CodeGen/Thumb2/mve-vcvt-fixed.ll:304
+
+define arm_aapcs_vfpcc <4 x float> @vcvt_i32_31(<4 x i32> %0) {
+; CHECK-LABEL: vcvt_i32_31:
----------------
dmgreen wrote:
> Can you add a testcase that uses 0x3E00000000000000 as the constant too. Plus another one that perhaps uses 0xBF00000000000000. That should cover a few negative power of 2 cases.
I certainly can


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

https://reviews.llvm.org/D103903