[llvm-commits] About: /llvm/trunk: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp test/Transforms/InstCombine/bit-checks.ll

[Dirk Steinke]

Hi everyone,

just my two bits about Owens patch to InstCombineAndOrXor.cpp.
The culprit should be the first line of the second transformation:

// (icmp eq (A & C1), 0) | (icmp eq (A & C2), 0) -->
// (icmp ne (A & (C1|C2)), (C1|C2)) where C1 and C2 are non-zero POT
if (LHSCC == ICmpInst::ICMP_EQ && LHSCst->isZero()) {
   Value *Op1 = 0, *Op2 = 0;

It is missing the "LHSCst == RHSCst && LHSCC == RHSCC" part.

The transformations themselves can't be wrong. They are actually too
specific.
The transformations should be
    (icmp eq (A & C1), C1) & (icmp eq (A & C2), C2)
     --> (icmp eq (A & (C1|C2)), (C1|C2))

    (icmp eq (A & C1), 0) & (icmp eq (A & C2), 0)
     --> (icmp eq (A & (C1|C2)), 0)

    (icmp ne (A & C1), C1) | (icmp ne (A & C2), C2)
     --> (icmp ne (A & (C1|C2)), (C1|C2))

    (icmp ne (A & C1), 0) | (icmp ne (A & C2), 0)
     --> (icmp ne (A & (C1|C2)), 0)

The current state only handles the special case, where C1 and
C2 only have a single bit set, so that
    (icmp eq (A & C1), C1)  iff  (icmp ne (A & C1), 0)
    (icmp ne (A & C1), C1)  iff  (icmp eq (A & C1), 0)

And: C1 and C2 don't have to be constant for those transformations to
work.

Another transformation not handled yet
    (icmp eq (A & C1), A) & (icmp eq (A & C2), A)
     --> (icmp eq (A & (C1&C2)), A)

I'll try to provide a patch, that handles the generic cases
    (icmp eq (A & B), C) & (icmp eq (A & D), E)
and
    (icmp ne (A & B), C) | (icmp ne (A & D), E)
when possible, in a couple of days. At least for the special
case (C == 0 || C == B) && (E == 0 || E == D), or when
B, C, D, and E are all constant.

bye
Dirk