[PATCH] [AArch64] Add v8.1a RDMA extension

[Tim Northover]

Hi Vladimir,

I think this mostly looks fine (very nice first patch!). I've got a couple of questions though (don't take them as non-negotiable, if you think you have good reasons for those decisions).

Cheers.

Tim.


================
Comment at: include/llvm/IR/IntrinsicsAArch64.td:660
@@ +659,3 @@
+// Vector Signed Saturating Rounding Doubling Multiply Accumulate Returning High Half
+def int_aarch64_neon_sqrdmlah : AdvSIMD_2IntArg_Intrinsic;
+// Vector Signed Saturating Rounding Doubling Multiply Subtract Returning High Half
----------------
I don't think these new intrinsics are needed. The instructions are effectively "(int_aarch64_neon_sqadd $acc, (int_aarch64_neon_sqrdmulh $LHS, $RHS))".

The only reason we've fused operations like that in the past is when loop optimisations can interfere and make block-at-a-time selection produce worse code.

Basically, this is when sext/zext happen *before* the key NEON op, and those nodes get hoisted out and become invisible to local selection (e.g. the int_aarch64_neon_smull intrinsics). That shouldn't be an issue here.

================
Comment at: lib/Target/AArch64/AArch64.td:29-30
@@ -28,1 +28,4 @@
 
+def FeatureRDMA: SubtargetFeature<"rdma","HasRDMA","true",
+  "Enable Advanced SIMD instruction extensions",[FeatureNEON]>;
+
----------------
Just how granular is v8.1? Are these instructions optional, or are we expecting CPUs that support them but not v8.1 in general?

If not, it'd be much better to keep things as hierarchical as possible and just add a HasV8_1 predicate, or perhaps FeatureFPARMv8_1, but again unless you expect a v8.1 core without a v8.1 FPU that could just be "Requires<HasV8_1, FeatureFPARMv8>".

http://reviews.llvm.org/D7998

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