[PATCH] D113095: Combine FADD and FMUL aarch64 intrinsics to FMLA

[Peter Waller via Phabricator via llvm-commits]

peterwaller-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:706
+  if (!match(FMul, m_Intrinsic<Intrinsic::aarch64_sve_fmul>(
+                       m_Deferred(p), m_Value(b), m_Value(c))))
+    return None;
----------------
Use `m_Specific`; `m_Deferred` is for when you want to match an earlier `m_Value`.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:714
+  llvm::FastMathFlags FMulFlags = cast<CallInst>(FMul)->getFastMathFlags();
+  // Don't combine when FMul & Fadd flags differ to prevent the loss of any
+  // additional important flags
----------------



================
Comment at: llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-fmla.ll:2
+; RUN: opt -S -instcombine < %s | FileCheck %s
+
+target triple = "aarch64-unknown-linux-gnu"
----------------
Some ideas:
1. Maybe generate the tests using llvm/utils/update_test_checks.py
2. How about calling this file sve-intrinsic-muladd, so that we don't have to introduce a new file for each of FMLA, FMAD, FMLS etc?
3. (Less important) I think I'd prefer to see names rather than anonymous instructions, so %p, %a, %b, etc. Makes it easier to conceptually line them up and means they don't have to be all renumbered should the test have something added to it in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113095