[PATCH] D111638: [AArch64][SVE] Combine predicated FMUL/FADD into FMA

[Peter Waller via Phabricator via llvm-commits]

peterwaller-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:700
+                                                        IntrinsicInst &II) {
+  Value *p, *a, *b, *c;
+  auto m_SVEFAdd = [](auto p, auto w, auto x) {
----------------
Please add a comment around your combine of the form `// fold (fadd a (fmul b c)) -> (fma a b c)`


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:718
+  if (!match(AddOp1, m_Intrinsic<Intrinsic::aarch64_sve_fmul>()))
+    return None;
+
----------------
peterwaller-arm wrote:
> I'd expect this all to look simpler, please have a go at simplifying. I think you can drop the matching logic above and instead add a condition that checks the AddOp1's predicate matches the Add's predicate.
> 
> One issue I have is that both the swap and m_SVEFAdd as it currently is serve the purpose of testing both operand orders. I think it's important for clarity to only do this once.
If you want to use the match syntax above I think you can also bind the fmul with `m_And(m_Value(FMul), m_SVEFMul(m_Deferred(p), m_Value(a), m_Value(b))` and then you could grab `FMul` and `c` simultaneously with the existing logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111638