[PATCH] D96424: [AArch64][SVE] Add patterns to generate FMLA/FMLS/FNMLA/FNMLS/FMAD

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Sure, It would at least make sense to do things in machine combiner at a later date. We need to be sure we don't make anything else worse whilst we are doing it though!



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11553-11554
+  switch (ScalarVT.getSimpleVT().SimpleTy) {
+  case MVT::f16:
+    return Subtarget->hasFullFP16() && VT.isScalableVector();
   case MVT::f32:
----------------
This feels like a separate change. I don't see a reason to not fold to FMA when HasFullFP16 in general. (But that would require it's own tests).


================
Comment at: llvm/lib/Target/AArch64/AArch64SelectionDAGInfo.cpp:58
+  const auto &STI = DAG.getMachineFunction().getSubtarget<AArch64Subtarget>();
+  return (OptLevel >= CodeGenOpt::Aggressive) && !STI.hasSVE();
 }
----------------
This needs to be more precise. Enabling SVE should not regress scalar/Neon code.

It looks like it should be possible to add the VT to this definition.


================
Comment at: llvm/lib/Target/AArch64/AArch64SelectionDAGInfo.h:31-32
                                   bool ZeroData) const override;
-  bool generateFMAsInMachineCombiner(CodeGenOpt::Level OptLevel) const override;
+  bool generateFMAsInMachineCombiner(SelectionDAG &DAG,
+                                     CodeGenOpt::Level OptLevel) const override;
 };
----------------
Could we just make it so that AArch64Subtarget->TSInfo is passed the subtarget directly on construction? 


================
Comment at: llvm/test/CodeGen/AArch64/sve-fp-combine.ll:3
+
+; NOTE: -fp-contract=fast required for fmla
+
----------------
It's better, in the long run, to use fast math flags over the global fp-contract I believe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96424