[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:701
+  // fold (fadd p a (fmul p b c)) -> (fma p a b c)
+  Value *p = II.getOperand(0);
+  Value *a = II.getOperand(1);
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113095