[PATCH] D65808: [ARM] Select VFMA

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Remember to upload with context! It makes the reviews easier to read.



================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:2604
             (v8f16 (MVE_VADDf16 (v8f16 MQPR:$val1), (v8f16 MQPR:$val2)))>;
+  def : Pat<(v8f16 (fma (v8f16 QPR:$src1), (v8f16 QPR:$src2), (v8f16 QPR:$src3))),
+            (v8f16 (MVE_VFMAf16 $src3, $src1, $src2))>;
----------------
I think this is near VADD? Do you mind moving it up to near VFMA, to keep things together.


================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:2606
+            (v8f16 (MVE_VFMAf16 $src3, $src1, $src2))>;
+  def : Pat<(v4f32 (fma (v4f32 QPR:$src1), (v4f32 QPR:$src2), (v4f32 QPR:$src3))),
+            (v4f32 (MVE_VFMAf32 $src3, $src1, $src2))>;
----------------
Also I think it's better to use MQPR, over QPR


================
Comment at: llvm/test/CodeGen/ARM/vfma.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=thumbv8.1m.main-arm-none-eabi -mattr=+mve.fp %s -o - | FileCheck %s
+
----------------
Can you move this test to test/CodeGen/Thumb2/mve-vfma.ll? Just to be with the others


================
Comment at: llvm/test/CodeGen/ARM/vfma.ll:24
+
+; Function Attrs: nounwind readnone speculatable
+declare <4 x float> @llvm.fma.v4f32(<4 x float>, <4 x float>, <4 x float>) #1
----------------
These lines can go,  as a little bit of a cleanup.


================
Comment at: llvm/test/CodeGen/ARM/vfma.ll:25
+; Function Attrs: nounwind readnone speculatable
+declare <4 x float> @llvm.fma.v4f32(<4 x float>, <4 x float>, <4 x float>) #1
+; Function Attrs: nounwind readnone speculatable
----------------
As can the #1 on the end of these.


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

https://reviews.llvm.org/D65808