[PATCH] ARM: Enable DP copy, load and store instructions for FPv4-SP

Renato Golin renato.golin at linaro.org
Wed Aug 20 05:38:07 PDT 2014


Hi Oliver,

Overall a good patch. I have a few comments inline and I also agree with Tim that this could actually be lots of smaller patches.

Feel free to address the comments here, but apply them (where applicable) to the individual patches later on.

cheers,
--renato

================
Comment at: lib/Target/ARM/ARMCallingConv.h:238
@@ -237,9 +237,3 @@
     unsigned Size = LocVT.getSizeInBits() / 8;
-    unsigned Align = Size;
-
-    if (LocVT.SimpleTy == MVT::v2f64 || LocVT.SimpleTy == MVT::i32) {
-      // Vectors are always aligned to 8 bytes. If we've seen an i32 here
-      // it's because it's been split from a larger type, also with align 8.
-      Align = 8;
-    }
+    unsigned Align = LocVT.SimpleTy == MVT::v2f64 ? 8 : Size;
 
----------------
Wild guess here, but shouldn't this be something like std::min(Size, 8)?

================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:3268
@@ -3235,2 +3267,3 @@
                              SDLoc dl) const {
+  assert(!Subtarget->isFPOnlySP() || RHS.getValueType() != MVT::f64);
   SDValue Cmp;
----------------
Is this the only such assert, or just the one that you hit during tests?

================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:3492
@@ +3491,3 @@
+                                   SDValue Cmp, SelectionDAG &DAG) const {
+  if (Subtarget->isFPOnlySP() && VT == MVT::f64) {
+    FalseVal = DAG.getNode(ARMISD::VMOVRRD, dl,
----------------
Would this ever be called by v2f64 types?

================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:8593
@@ -8482,1 +8592,3 @@
+static SDValue PerformVMOVDRRCombine(SDNode *N, SelectionDAG &DAG,
+                                     const ARMSubtarget *Subtarget) {
   // N=vmovrrd(X); vmovdrr(N:0, N:1) -> bit_convert(X)
----------------
This looks like left over.

================
Comment at: test/CodeGen/Thumb2/float-cmp.ll:3
@@ +2,3 @@
+; RUN: llc < %s -mtriple=thumbv7-none-eabihf -mcpu=cortex-m4 | FileCheck %s -check-prefix=CHECK -check-prefix=SP
+; RUN: llc < %s -mtriple=thumbv7-none-eabihf -mcpu=cortex-a8 | FileCheck %s -check-prefix=CHECK -check-prefix=DP
+
----------------
You could have used the same HARD+SP/DP you used in the test above to simplify this test.

http://reviews.llvm.org/D4907






More information about the llvm-commits mailing list