[PATCH] D15015: [AArch64] Add ARMv8.2-A FP16 vector instructions

[Oliver Stannard via llvm-commits]

olista01 marked 4 inline comments as done.
olista01 added a comment.

> What do you think of introducing more base classes for the H and non-H variants? For instance, for BaseSIMDThreeSameVector, you could have something like BaseSIMDThreeSameVectorH which just overrides (or passes up, if we also added a BaseSIMDThreeSameVectorSD) the correct bit. That could help avoid a lot of the noise, and keep the descriptions a little cleaner, I think.


I'm not sure about this. It would make this patch smaller, but would complicate the inheritance trees by having an extra layer of base classes for the FP instructions (which have different parameters), which I don't think would be clearer in the end.

> Another thing is, many of the patterns are in standalone multiclasses, and would need f16 variants as well. I'll submit a patch if you don't beat me to it!


I don't have any further patches for this (I just put patterns in where copying the existing ones was obviously correct), so go ahead.


================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:5700
@@ -5625,3 +5699,3 @@
 
-multiclass SIMDThreeScalarSD<bit U, bit S, bits<5> opc, string asm,
+multiclass SIMDThreeScalarSD<bit U, bit S, bits<3> opc, string asm,
                              SDPatternOperator OpNode = null_frag> {
----------------
ab wrote:
> SIMDThreeScalarSD -> SIMDThreeScalarHSD?
> Or, as we do elsewhere -better IMO-, SIMDFPThreeScalar?
I also prefer the latter, since *HSD is still wrong for targets without FullFP16.

================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:6053
@@ -5948,3 +6052,3 @@
 
 multiclass SIMDAcrossLanesS<bits<5> opcode, bit sz1, string asm,
                             Intrinsic intOp> {
----------------
ab wrote:
> SIMDAcrossLanesS -> HS ?
Changed to SIMDFPAcrossLanes


Repository:
  rL LLVM

http://reviews.llvm.org/D15015