[PATCH] D76439: [SDAG] fix crash in getNegatedExpression() from altered number of uses

[qshanz via Phabricator via llvm-commits]

steven.zhang added a comment.

In D76439#1937652 <https://reviews.llvm.org/D76439#1937652>, @spatel wrote:

> In D76439#1935992 <https://reviews.llvm.org/D76439#1935992>, @steven.zhang wrote:
>
> > We need to update the cost as they are pair. For now, it returns the first operand immediately if it is not expensive. But it could be neutral. And we will have problems if the cost of second operand is cheap. Because we should return cheap, but now, it is neutral. Does it make sense ?
>
>
> Sorry, but I'm not understanding. With this patch we have fneg (fmul/fdiv X, Y):
>
> 1. CostX == cheaper == 2; CostY == cheaper == 2 --> negate X
> 2. CostX == cheaper == 2; CostY == neutral == 1 --> negate X
> 3. CostX == neutral == 1; CostY == cheaper ==2 --> negate Y
> 4. CostX == neutral == 1; CostY == neutral == 1 --> negate X
>
>   But I have an alternate idea to avoid the crash that is probably a better fix given the current logic: D76638 <https://reviews.llvm.org/D76638>


I mean when evaluating the cost of fmul/fdiv X, Y, not the logic inside getNegatedExpression, but the logic inside getNegatibleCost. We have:

- CostX == neutral == 1; CostY == cheaper --> Cost(fmul/fdiv) == neutral (**Wrong**)

This is the code.

  case ISD::FMUL:
  case ISD::FDIV: {
    // fold (fneg (fmul X, Y)) -> (fmul (fneg X), Y) or (fmul X, (fneg Y))
    NegatibleCost V0 = getNegatibleCost(Op.getOperand(0), DAG, LegalOperations,
                                        ForCodeSize, Depth + 1);
    if (V0 != NegatibleCost::Expensive)    // Steven: if it is neutral, we should not return immediately, we need to check the cost of operand 2 and compare them.
      return V0;
  
    // Ignore X * 2.0 because that is expected to be canonicalized to X + X.
    if (auto *C = isConstOrConstSplatFP(Op.getOperand(1)))
      if (C->isExactlyValue(2.0) && Op.getOpcode() == ISD::FMUL)
        return NegatibleCost::Expensive;
  
    return getNegatibleCost(Op.getOperand(1), DAG, LegalOperations, ForCodeSize,
                            Depth + 1);


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

https://reviews.llvm.org/D76439