[llvm-commits] [llvm] r160101 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp test/Transforms/InstCombine/2012-07-11-AddSubDemandedBits.ll

[Evan Cheng]

I've backed out this patch and implemented the dag combine to recover in codegen: r160387. I still question the usability of the instcombine transformation though.

Evan

On Jul 16, 2012, at 11:41 PM, Evan Cheng wrote:

> 
> On Jul 15, 2012, at 4:19 PM, Chris Lattner <clattner at apple.com> wrote:
> 
>> On Jul 13, 2012, at 2:33 AM, Nick Lewycky wrote:
>>> Evan Cheng wrote:
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=160101&view=rev
>>>> Log:
>>>> Instcombine was transforming:
>>>> %shr = lshr i64 %key, 3
>>>> %0 = load i64* %val, align 8
>>>> %sub = add i64 %0, -1
>>>> %and = and i64 %sub, %shr
>>>> ret i64 %and
>>>> 
>>>> to:
>>>> %shr = lshr i64 %key, 3
>>>> %0 = load i64* %val, align 8
>>>> %sub = add i64 %0, 2305843009213693951
>>>> %and = and i64 %sub, %shr
>>>> ret i64 %and
>>>> 
>>>> The demanded bit optimization is actually a pessimization because add -1 would
>>>> be codegen'ed as a sub 1. Teach the demanded constant shrinking optimization
>>>> to check for negated constant to make sure it is actually reducing the width
>>>> of the constant.
>>> 
>>> I'm not a fan of this patch. Instead of just disabling the optz'n in 
>>> this case, we should have some way to transform things into the '%sub = 
>>> add i64 %0, -1' form, more generally.
>> 
>> I agree with Nick.  SelectionDAG should be able to handle this sort of thing - it already does handle matching and/or masks that have been simplified away.  Can't we use similar logic for subtract?
> 
> I considered handling this in codegen but I simply didn't see any good reason for this transformation in this first place. It's not an optimization for any targets. It doesn't make the IR easier to understand and I don't see it enabling other optimizations.
> 
> Please elaborate on where you can think this can be handled in codegen.This is not a case where selectdag can reason about the add by itself.  DAG combine can potentially do recognize something like:
> (and (add x, c1), (lshr y, c2)) where # of leading zeros of c1 is equal to c2
> to recover from this. But it needs to know whether the constant can fit in the instruction and which constants is *better*. It seems silly to transform it back and forth for no apparent benefit.
> 
> Evan
> 
>> 
>> -Chris
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits