[PATCH] [AArch64] Add v8.1a "Rounding Double Multiply Add/Subtract" extension

[James Molloy]

Hi Vladimir,

Thanks for working on this. It looks really good!

Comments inline.

Cheers,

James


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:8555
@@ -8523,1 +8554,3 @@
 //----------------------------------------------------------------------------
+// AdvSIMD v8.1 Rounding Double Multiply Add/Subtract
+//----------------------------------------------------------------------------
----------------
I think this should say "Doubling", right?

================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:8607
@@ +8606,3 @@
+  }
+  // FIXME: uncomment the following, after backend will support i16 neon type
+  //def : Pat<(i16 (Accum (i16 FPR16Op:$Rd),
----------------
We don't allow commented out code, sorry. You should also be able to enable this pattern, by using an SMOV instruction (you'd need a UMOV for the equivalent unsigned operation):

    def : Pat<(... stays the same ...),
                  (SMOVvi16to32 (!cast<Instruction>(NAME # v4i16_indexed)
                    ..., VectorIndexH:0)>;

But the commented out pattern seems weird; it uses SUBREG_TO_REG, which I don't understand why, and it matches sqdmull, not sqrdmulh like the surrounding code. So something's fishy here.

================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:8657
@@ +8656,3 @@
+                        (i32 (vector_extract 
+                               (v4i32 (insert_subvector
+                                        (undef), 
----------------
This is really weird and sounds like a bug, although if the pattern matches I can't really argue with it as it means the bug is somewhere else...

... I assume this pattern is explicitly tested?

================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:8714
@@ +8713,3 @@
+    [
+  // FIXME: uncomment the following, after backend will support i16 neon type
+  //  (set (i16 FPR16Op:$dst),
----------------
You should be able to implement this with SMOV/UMOV, as I mentioned above.

================
Comment at: lib/Target/AArch64/AArch64InstrInfo.td:3086
@@ +3085,3 @@
+  // FIXME: uncomment the following, after backend will support i16 neon type
+  //def : Pat<(i16 (int_aarch64_neon_sqadd
+  //                 (i16 FPR16:$Rd),
----------------
Again, UMOV/SMOV to implement this.

http://reviews.llvm.org/D8502

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/