[TargetLowering] Avoid infinite iteration on setcc fold

[greened]

Summary

When folding (X & Y) == Y to (~X & Y) == 0, we should only do the fold if the computed zero value is not Y. Otherwise, the resulting compare will match the same pattern ad infinitum.

[greened]

This appeared using a quite old version of LLVM with an out-of-tree target and I'm having trouble getting a valid testcase out that runs with the latest LLVM/bugpoint. The code in question is exacly the same in the old and current LLVM so the bug still exists currently but I haven't yet been able to generate a testcase. Thought I would at least put this up for review. I'm not entirely confident current master could get into this situation but it seems wise to guard against it.

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

2967

Is this happening with vector cases, in which case would this work instead:

ConstantSDNode *YConst = isConstOrConstSplat(Y) ?

[greened]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

2967

That would work for the case I saw. But it seems prudent to guard against future constructs which might appear. Anything that results in Y == Zero is going to cause an infinite loop.

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

2967

Under what circumstances will the isConstOrConstSplat + isNullValue() fail but the comparison against Zero work?

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

2967

@greened - I'm not sure if you're still looking at this, but by inspection this should work. Any luck on creating a test case?

ConstantSDNode *YConst =
    isConstOrConstSplat(Y, /*AllowUndefs*/ true, /*AllowTruncation*/ true);
if (YConst && YConst->isNullValue())
  return SDValue();
assert(Y != Zero && "Unknown zero value");

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

if (DAG.isKnownToBeAPowerOfTwo(Y)) {
     // If the target supports an 'and-not' or 'and-complement' logic operation,
     // try to use that to make a comparison operation more efficient.
     // But don't do this transform if the mask is a single bit because there are
     // more efficient ways to deal with that case (for example, 'bt' on x86 or
     // 'rlwinm' on PPC).
 
     // Bail out if the compare operand that we want to turn into a zero is
     // already a zero (otherwise, infinite loop).
     auto *YConst = dyn_cast<ConstantSDNode>(Y);
     if (YConst && YConst->isNullValue())
       return SDValue();
 
+    // Bail out if Y is already equivalent to Zero.  Otherwise we will
+    // infinitely recurse as (X' & Y) == Y (where X' is ~X) still matches the
+    // pattern (X & Y) == Y, where X = X'.
+    if (Y == Zero)
+      return SDValue();
+
     // Transform this into: ~X & Y == 0.
     SDValue NotX = DAG.getNOT(SDLoc(X), X, OpVT);
     SDValue NewAnd = DAG.getNode(ISD::AND, SDLoc(N0), OpVT, NotX, Y);
     return DAG.getSetCC(DL, VT, NewAnd, Zero, Cond);
   }
 
   return SDValue();
 }