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

[David Majnemer]

On Tue, May 31, 2011 at 2:53 AM, David Majnemer
<david.majnemer at gmail.com> wrote:
> Hi Duncan,
>
> On Tue, May 31, 2011 at 2:28 AM, Duncan Sands <baldrick at free.fr> 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.
> You are correct, I somehow transformed this in my head to if ((x & C)
> != C) x |= C, the transform must be more strict.
>
>>
>> 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.
> Just realized that those comments are wrong, it should be (x & y) != 0....
>
> Yes, I also assumed that m_Not would match ConstantInt, however it
> does not. I considering making the change you recommended but decided
> that m_Not is used in quite a few places and changing the semantics of
> m_Not in this way would slow it down and potentially break things. I
> am guessing that most uses of m_Not that would hit ConstantInt would
> be smashed away by constant folding instead of being this case... With
> that in mind I am open to modifying m_Not, I just don't know how much
> the consumers of that match would like it.
>
> With that in mind "if ((x & y) != 0) x &= ~y" ==> "x &= ~y" is wrong
> while "if ((x & y) == 0) x &= ~y" ==> "x" is OK
>
>>
>> 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.
> Hrm, that does sound useful but it also seems like a serious reworking
> of InstructionSimplify...
>>
>> Ciao, Duncan.
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
I have attached something that is correct save for any further issues.
The only thing lingering is the mechanism to reason about simple
implications... If it is necessary, a basic list of what is needed
would be nice :)

-- 
David Majnemer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: instcombine2.patch
Type: application/octet-stream
Size: 5603 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110531/bf3a9cb0/attachment.obj>