[PATCH] D70975: [SDAG] remove use restriction in isNegatibleForFree() when called from getNegatedExpression()

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D70975#1780280 <https://reviews.llvm.org/D70975#1780280>, @spatel wrote:

> In D70975#1780237 <https://reviews.llvm.org/D70975#1780237>, @fhahn wrote:
>
> > This seems to cause a DAGCombiner to not terminate for some inputs on AArch64. Unfortunately I cannot share the inputs that cause the cycle :(
>
>
> Thanks for letting us know. Can you check if D70595 <https://reviews.llvm.org/D70595> would cause the same cycle?
>  Will it be possible to reduce a test that can be shared? I'm not sure how to proceed without a test.


I can't tell from this log, but it's possible that this bot has infinite looped because of this change?
http://lab.llvm.org:8011/builders/clang-s390x-linux-lnt/builds/15964

If we need to revert and D70595 <https://reviews.llvm.org/D70595> would also cause problems, I have a one-line hack that could work-around the crashing that motivated this patch and hopefully not induce any other significant diffs:

  diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  index f8afdaf086a..b85150fb7cd 100644
  --- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  +++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  @@ -5619,7 +5619,12 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
                                    ForCodeSize, Depth + 1);
       char V1 = isNegatibleForFree(Op.getOperand(1), DAG, LegalOperations,
                                    ForCodeSize, Depth + 1);
  -    if (V0 >= V1) {
  +    // TODO: It is possible that costs have changed between now and the initial
  +    //       calls to isNegatibleForFree(). That is because we are rewriting the
  +    //       expression, and that may change the number of uses (and therefore
  +    //       the cost) of values. If the negation costs are equal, only negate
  +    //       this value if it is a constant. Otherwise, try operand 1.
  +    if (V0 > V1 || (V0 == V1 && isa<ConstantFPSDNode>(Op.getOperand(0)))) {
         // fold (fneg (fma X, Y, Z)) -> (fma (fneg X), Y, (fneg Z))
         SDValue Neg0 = getNegatedExpression(
             Op.getOperand(0), DAG, LegalOperations, ForCodeSize, Depth + 1);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70975