[llvm-commits] bug fix for infinite loop in InstCombine

[Guo, Xiaoyi]

Please review the attached fix for a bug in InstCombiner. The patch also includes a new test case which will cause the current InstCombiner to go into an infinite loop.

Please help to commit if acceptable.

The problem is with the following code in InstCombiner::visitAnd() in InstCombineAndOrXor.cpp:

Instruction *InstCombiner::visitAnd(BinaryOperator &I) {
  bool Changed = SimplifyAssociativeOrCommutative(I);
  ...
    if (Op0->hasOneUse() &&
        match(Op0, m_Xor(m_Value(A), m_Value(B)))) {
      if (A == Op1) {                                // (A^B)&A -> A&(A^B)
        I.swapOperands();     // Simplify below
        std::swap(Op0, Op1);				<==========
      } else if (B == Op1) {                         // (A^B)&B -> B&(B^A)
        cast<BinaryOperator>(Op0)->swapOperands();
        I.swapOperands();     // Simplify below
        std::swap(Op0, Op1);				<==========
      }
    }

    if (Op1->hasOneUse() &&
        match(Op1, m_Xor(m_Value(A), m_Value(B)))) {
      if (B == Op0) {                                // B&(A^B) -> B&(B^A)
        cast<BinaryOperator>(Op1)->swapOperands();
        std::swap(A, B);
      }
      // Notice that the patten (A&(~B)) is actually (A&(-1^B)), so if
      // A is originally -1 (or a vector of -1 and undefs), then we enter
      // an endless loop. By checking that A is non-constant we ensure that
      // we will never get to the loop.
      if (A == Op0 && !isa<Constant>(A)) // A&(A^B) -> A & ~B
        return BinaryOperator::CreateAnd(A, Builder->CreateNot(B, "tmp"));
    }


With the given test case, op0 is a const and op1 is xor, so the two operands are be swapped in SimplifyAssociativeOrCommutative(). Then I's operands are swapped back at one of the statements marked "<===========" above. Then because A is a constant, a new instruction is not created. So we end up with Changed flag as true but the instruction is not changed and we go into an infinite loop in InstCombiner::DoOneIteration().

My fix is to remove the swapOperands() calls, because I don't think they are necessary.

Thanks,
Xiaoyi
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: diff.txt
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110915/1f236329/attachment.txt>