[PATCH] D99074: [llvm][AArch64][SVE] Fold literals into math instructions

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

Structurally this is looking great.  There is just a few missing patterns after which I think you're done.



================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:498
+: Pat<(vt (op (pt PPR_3b:$Pg), (vt ZPR:$Zs1), (vt (AArch64dup (it immL))))),
+      (inst PPR_3b:$Pg, ZPR:$Zs1, imm)>;
+
----------------
Does the `inst` line need the `PPR_3b:` and `ZPR:` parts? I ask because the next class you've added seems to work without them.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:1878-1883
+  def : SVE_2_Op_Fp_Imm_Pat<nxv8f16, ir_op, nxv8i1, f16, A, 0, !cast<Instruction>(NAME # "_UNDEF_H")>;
+  def : SVE_2_Op_Fp_Imm_Pat<nxv8f16, ir_op, nxv8i1, f16, B, 1, !cast<Instruction>(NAME # "_UNDEF_H")>;
+  def : SVE_2_Op_Fp_Imm_Pat<nxv4f32, ir_op, nxv4i1, f32, A, 0, !cast<Instruction>(NAME # "_UNDEF_S")>;
+  def : SVE_2_Op_Fp_Imm_Pat<nxv4f32, ir_op, nxv4i1, f32, B, 1, !cast<Instruction>(NAME # "_UNDEF_S")>;
+  def : SVE_2_Op_Fp_Imm_Pat<nxv2f64, ir_op, nxv2i1, f64, A, 0, !cast<Instruction>(NAME # "_UNDEF_D")>;
+  def : SVE_2_Op_Fp_Imm_Pat<nxv2f64, ir_op, nxv2i1, f64, B, 1, !cast<Instruction>(NAME # "_UNDEF_D")>;
----------------
For this class, which deals the the normal IR nodes (i.e. not intrinsics), can you add entries for all the legal floating point types (i.e. add entries for `nxv2f16, nxv4f16 & nxv2f32`) along with suitable tests.


================
Comment at: llvm/test/CodeGen/AArch64/sve-fp-immediates-merging.ll:22-33
+define <vscale x 8 x half> @fadd_h_immhalf_acle(<vscale x 8 x i1> %pg, <vscale x 8 x half> %a) #0 {
+; CHECK-LABEL: fadd_h_immhalf_acle:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    fadd z0.h, p0/m, z0.h, #0.5
+; CHECK-NEXT:    ret
+  %elt   = insertelement <vscale x 8 x half> undef, half 0.500000e+00, i32 0
+  %splat = shufflevector <vscale x 8 x half> %elt, <vscale x 8 x half> undef, <vscale x 8 x i32> zeroinitializer
----------------
Sorry to be a pain but can you move the intrinsic tests into a separate file.  This is what we've done for some of the other ones, for example there's `sve-intrinsics-int-arith.ll` and then `sve-intrinsics-int-arith-imm.ll`.  So can you move them into `sve-intrinsics-fp-arith-imm.ll` leaving this file to just contain the stock IR tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99074/new/

https://reviews.llvm.org/D99074