[PATCH] D140181: DAG: Pull fneg out of select feeding fadd into fsub

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7013
+    SDValue RHS = Op.getOperand(2);
+    if (LHS.getOpcode() == ISD::FNEG && RHS.getOpcode() == ISD::FNEG) {
+      return DAG.getNode(ISD::SELECT, DL, VT, Op.getOperand(0),
----------------
arsenm wrote:
> arsenm wrote:
> > RKSimon wrote:
> > > Why don't you want to use getNegatedExpression ?
> > This is in getNegatedExpression and it didn’t occur to be to use it recursively 
> Of course the first case I try with this infinite loops
I can't figure out how to represent this with the costs here. If I have this:


```
define float @fadd_select_fsub_fsub_f32(i32 %arg0, float %x, float %y, float %z) {
  %cmp = icmp eq i32 %arg0, 0
  %neg.x = fsub nsz float %x, 2.0
  %neg.y  = fsub nsz float %y, 2.0
  %select = select i1 %cmp, float %neg.x, float %neg.y
  %add = fadd float %select, %z
  ret float %add
}

```

I'd expect to swap the fsubs and turn the fadd into an fsub. If I report Cheaper here, that happens. But then, the new fsub is turned right back into the fadd. The cost is only obviously cheaper if the cost of the negate in the select operands is canceled out by the negate of the fadd user. I'm not sure whether I should try to be hacking either the fadd or fsub combines, or making the cost system more sophisticated.

I also don't understand why  (fneg (fsub X, Y)) -> (fsub Y, X) is only considered neutral, and not cheaper if fsub is legal



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

https://reviews.llvm.org/D140181