[PATCH] D46311: [AArch64] added FP16 vcvth intrinsic support

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:7793
+multiclass SIMDFPScalarRShift<bit U, bits<5> opc, string asm,
+                              SDPatternOperator OpNode=null_frag> {
+
----------------
Do we use OpNode?


================
Comment at: lib/Target/AArch64/AArch64InstrFormats.td:7805
+  def HDr : BaseSIMDScalarShift<U, opc, {?,?,?,?,?,?,?},
+                              FPR16, FPR64, vecshiftR16, asm, []>{
+    let Inst{21-16} = imm{5-0};
----------------
Nit1: spaces are off: FPR16 should be aligned under U.
Nit2: space between ">{". Same for other rules below.


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.td:4869
 //----------------------------------------------------------------------------
-defm FCVTZS : SIMDFPScalarRShift<0, 0b11111, "fcvtzs">;
-defm FCVTZU : SIMDFPScalarRShift<1, 0b11111, "fcvtzu">;
-defm SCVTF  : SIMDFPScalarRShift<0, 0b11100, "scvtf">;
-defm UCVTF  : SIMDFPScalarRShift<1, 0b11100, "ucvtf">;
+defm FCVTZS : SIMDFPScalarRShift<0, 0b11111, "fcvtzs", int_aarch64_neon_vcvtfp2fxs>;
+defm FCVTZU : SIMDFPScalarRShift<1, 0b11111, "fcvtzu", int_aarch64_neon_vcvtfp2fxu>;
----------------
Do we need to pass the intrinsics opnodes?


Repository:
  rL LLVM

https://reviews.llvm.org/D46311