[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:724
+  llvm::FastMathFlags flags = II.getFastMathFlags();
+  flags &= FMulInst->getFastMathFlags();
+
----------------
peterwaller-arm wrote:
> MattDevereau wrote:
> > peterwaller-arm wrote:
> > > peterwaller-arm wrote:
> > > > It seems to me that a check is needed if the fast math flags contain 'contract', and if not, bailout.
> > > Please also check that the flags are equal instead of taking their intersection. The fadd might plausibly have a flag which allows more aggressive optimization and contracting it in this way may prevent those optimizations from taking place.
> > I'm not sure what you mean here. Since flags is its own fresh variable isnt the intersection the same as comparing if the flags are equal? The original flags on the fadd intrinsic will be preserved
> When I wrote 'contracting' I think I was thinking 'intersecting'.
> 
> My suggestion is to do `if (flags1 == flags2) return None;`, which cannot be the same as doing an intersection -- in my proposed case the optimization would not take place. Intersection implies constructing a new set of flags which is different from those flags sets on the input. Also not sure what you mean by 'the original flags on the fadd intrinsic will be preserved' -- we're expecting that this optimization will replace the fadd with the newly constructed fmul, so the fadd is going to be erased.
Sorry, I meant if (flags1 != flags2) return None;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111638