[PATCH] D113095: Combine FADD and FMUL aarch64 intrinsics to FMLA
Peter Waller via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 3 08:47:00 PDT 2021
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:
> 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
FMLA is an acronym so warrants full capitalization, FMul is more like PTest, and you can find other examples of things being called Mul. It's camel case, so I preferred it as you had it before (there are other examples in the file of 'Mul' for example).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113095/new/
https://reviews.llvm.org/D113095
More information about the llvm-commits
mailing list