[PATCH] D75157: [DAGCombine] Perform the fold of A - (-B) -> A + B only when it is cheaper

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D75157#1893398 <https://reviews.llvm.org/D75157#1893398>, @lebedev.ri wrote:

> In D75157#1892994 <https://reviews.llvm.org/D75157#1892994>, @steven.zhang wrote:
>
> > In D75157#1892866 <https://reviews.llvm.org/D75157#1892866>, @lebedev.ri wrote:
> >
> > > The test changes don't immediately stand out to me as improvements.
> > >  I'd think the fix should go in other direction.
> >
> >
> > I want to make them consistent to avoid the confusion no matter which direction.
>
>
>
>
> In D75157#1893099 <https://reviews.llvm.org/D75157#1893099>, @RKSimon wrote:
>
> > Loss of commutativity by going from fadd to fsub will likely cause register pressure regressions.
>
>
> I'm not very active wrt fp side of things, but i would almost think we need something like this instead
>
>   // unfold (fsub A, B) -> (fadd A, (fneg B))
>   if ((!LegalOperations || TLI.isOperationLegalOrCustom(ISD::FADD, VT)) &&
>       TLI.getNegatibleCost(N1, DAG, LegalOperations, ForCodeSize) !=
>           TargetLowering::NegatibleCost::Expensive)
>     return DAG.getNode(
>         ISD::FADD, DL, VT, N0,
>         TLI.getNegatedExpression(N1, DAG, LegalOperations, ForCodeSize), Flags);
>


Right, and we do that already.
Then i'm not sure there is anything to fix here..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75157