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

[Matt Devereau via Phabricator via llvm-commits]

MattDevereau marked 7 inline comments as done.
MattDevereau added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:701
+  // fold (fadd p a (fmul p b c)) -> (fma p a b c)
+  Value *p = II.getOperand(0);
+  Value *a = II.getOperand(1);
----------------
peterwaller-arm wrote:
> MattDevereau wrote:
> > paulwalker-arm wrote:
> > > These should be upper case to match LLVM styling and be consistent with the other names using within this patch.
> > I'm not 100% sure what you mean by "these" being consistent with the other names
> > 
> > p -> P
> > a -> A
> > b -> B
> > c -> C
> > FMul -> FMUL?
> > 
> Paul is referring to this section of the style guide: https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> 
> I think `FMul` is in keeping with the surroundings as it is. Your other suggestions look fine to me in the context, or you could name them more thoroughly (Pred/Predicate, OpA, OpB, OpC) for example.
I've changed FMul to FMUL to stay in line with FMLA further down in the function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113095