[llvm-commits] [llvm] r147749 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp test/Transforms/InstCombine/sign-test-and-or.ll

[Eli Friedman]

On Sun, Jan 8, 2012 at 10:32 AM, Benjamin Kramer
<benny.kra at googlemail.com> wrote:
> Author: d0k
> Date: Sun Jan  8 12:32:24 2012
> New Revision: 147749
>
> URL: http://llvm.org/viewvc/llvm-project?rev=147749&view=rev
> Log:
> InstCombine: If we have a bit test and a sign test anded/ored together, merge the sign bit into the bit test.
>
> This is common in bit field code, e.g. checking if the first or the last bit of a bit field is set.
>
> Modified:
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
>    llvm/trunk/test/Transforms/InstCombine/sign-test-and-or.ll
>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp?rev=147749&r1=147748&r2=147749&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Sun Jan  8 12:32:24 2012
> @@ -743,6 +743,22 @@
>       }
>     }
>   }
> +
> +  // (X & C) == 0 & X > -1  ->  (X & (C | SignBit)) == 0
> +  if (LHS->hasOneUse() && RHS->hasOneUse() &&
> +      ((LHSCC == ICmpInst::ICMP_EQ && LHSCst->isZero() &&
> +        RHSCC == ICmpInst::ICMP_SGT && RHSCst->isAllOnesValue()) ||
> +       (RHSCC == ICmpInst::ICMP_EQ && RHSCst->isZero() &&
> +        LHSCC == ICmpInst::ICMP_SGT && LHSCst->isAllOnesValue()))) {
> +    BinaryOperator *BO =
> +      dyn_cast<BinaryOperator>(LHSCC == ICmpInst::ICMP_EQ ? Val : Val2);
> +    ConstantInt *AndCst;
> +    if (BO && match(BO, m_OneUse(m_And(m_Value(), m_ConstantInt(AndCst))))) {
> +      APInt New = AndCst->getValue() | APInt::getSignBit(AndCst->getBitWidth());
> +      BO->setOperand(1, ConstantInt::get(AndCst->getContext(), New));
> +      return BO == Val ? LHS : RHS;
> +    }
> +  }
>
>   // From here on, we only handle:
>   //    (icmp1 A, C1) & (icmp2 A, C2) --> something simpler.
> @@ -1442,6 +1458,22 @@
>     }
>   }
>
> +  // (X & C) != 0 & X < 0  ->  (X & (C | SignBit)) != 0

Err, shouldn't this be "(X & C) != 0 | X < 0"?  It looks like the
actual transformation is correct, just the comment is wrong.

On a more general note, could this code be combined with the code that
transforms "(X & C1) == 0 & (X & C2) == 0 -> (X & (C1|C2)) == 0"?
It's essentially the same transformation; it's just that there's a
more canonical way of writing "(X & SignBit) == 0".  Should be pretty
straightforward to write an "isAnyBitSetComparison()" helper.  (Could
also easily add some other potentially interesting transformations,
like "(X <u (1 << C1)) & ((X & C2) == 0)".)

-Eli