[llvm-commits] [pr1225] Fix using instcombine instead of instsimplify

[Eli Friedman]

2012/3/28 Rafael Espíndola <rafael.espindola at gmail.com>:
> I was able to reproduce the slowdown that Chad found. The problem was
> simply ComputeMaskedBits being too slow for instsimplify. When looking
> at where to add it to instcombine I was surprised that it already had
> code that should handle it:
>
> -----------------------------------------------------------------------------------------
>  case Instruction::And:
>    // If either the LHS or the RHS are Zero, the result is zero.
>   if (SimplifyDemandedBits(I->getOperandUse(1), DemandedMask,
>                             RHSKnownZero, RHSKnownOne, Depth+1) ||
>        SimplifyDemandedBits(I->getOperandUse(0), DemandedMask & ~RHSKnownZero,
>                             LHSKnownZero, LHSKnownOne, Depth+1))
>      return I;
> ...
>    // If all of the demanded bits are known 1 on one side, return the other.
>    // These bits cannot contribute to the result of the 'and'.
> ....
> ---------------------------------------------------------------------------------------
>
> It was not working because of the '& ~RHSKnownZero'.  The RHS was 1,
> so we would ask for information about the lsb bit and fail to see that
> all the other bits where known to be zero.
>
> It is very important that we pass the reduced mask to
> SimplifyDemandedBits since it enables it to do optimizations that are
> not valid for the other bits, so we cannot just drop the and.
>
> I tried adding a second call to ComputeMaskedBits with the original
> mask just after the calls to SimplifyDemandedBits. This worked, but
> still had a noticeable performance impact.
>
> What worked in the end was changing ComputeMaskedBits to always
> compute information for all the bits. That way we can keep passing the
> more restrictive mask to SimplifyDemandedBits and if it fails to
> simplify LHS it will still provide us with all known bit values.

My primary concern here is that this could introduce miscompiles
because some callers of ComputeMaskedBits or SimplifyDemanded might
get confused by seeing bits it didn't request/expect, like the issue I
fixed in r152066.  The concept of the patch seems fine, though.

-Eli