[llvm-commits] [PATCH] Implement a few missing InstCombine/InstSimplify optimizations

[Nick Lewycky]

Duncan Sands wrote:
> Hi David,
>
>> This patch implements a few of the optimizations mentioned in
>> http://www.nondot.org/sabre/LLVMNotes/InstCombine.txt
>>
>> specifically:
>> if ((x&   C) == 0) x |= C	becomes		x |= C
>> if ((x&   C) == 0) x&= ~C	becomes		nothing
>> if (((1<<   which_alternative)&   0x7)) becomes if (which_alternative<   3)
>> if (!((1<<   which_alternative)&   0x3)) becomes if (which_alternative>= 2)
>>
>> The patch also implements slightly more powerful variations of the
>> first two, C need not be a constant.
>
> unfortunately the first one is wrong if C is not a "single bit" (power of 2).
> Consider for example
>     if ((x&  C) == 0) x |= C
> Take x = 2 and C = 3.  Then x&C=2, so (x&C)==0 is false.  Thus the result is
> the original value of x, namely 2.  You propose replacing x with x|C in this
> case which is equal to 3.  So this changes the result from 2 to 3, which is
> wrong.
>
> Also, in this bit
>
> +    ConstantInt *AndCI, *And2CI;
> +    // Transform: "if ((x&  C) == 0) x&= ~C" ==>  x&= ~C
> +    if (match(AndRHS, m_ConstantInt(AndCI))&&
> +        match(TrueVal, m_And(m_Specific(FalseVal), m_ConstantInt(And2CI)))&&
> +        AndCI == ConstantExpr::getNot(And2CI)) {
> +      return FalseVal;
> +    }
> +    // Transform: "if ((x&  y) == 0) x&= ~y" ==>  x&= ~y
> +    if (match(TrueVal, m_And(m_Specific(FalseVal), m_Not(m_Specific(AndRHS))))) {
> +      return FalseVal;
> +    }
>
> the first part should be redundant, i.e. caught by the second part.  If it is
> not then I think you would do better to enhance m_Not to make the second part
> catch everything.

Careful that you don't cause an infinite loop in the optimizer. It would 
be fine in this case where the argument is an m_Specific, but it's not 
clear to me how implementable that is.

Nick

> That said, I don't much like these changes to InstructionSimplify.  What LLVM
> needs in general and InstructionSimplify in particular is a generic machinery
> for reasoning about simple implications.  Consider "select Cond, TV, FV".  If
> you can prove the implication "Cond =>  (TV == FV)" then you can replace the
> select with FV.  Your code above is a special case of this.  I would rather
> see a more general infrastructure for proving implications.
>
> Ciao, Duncan.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>