[llvm-commits] [llvm] r155136 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineShifts.cpp test/Transforms/InstCombine/2010-11-01-lshr-mask.ll test/Transforms/InstCombine/apint-shift.ll test/Transforms/InstCombine/cast.ll test/Transforms/In

Jakob Stoklund Olesen stoklund at 2pi.dk
Thu Apr 19 17:05:34 PDT 2012


On Apr 19, 2012, at 2:27 PM, Nick Lewycky <nlewycky at google.com> wrote:

> On 19 April 2012 14:08, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
> 
> On Apr 19, 2012, at 11:40 AM, Nick Lewycky <nlewycky at google.com> wrote:
> 
>> On 19 April 2012 09:46, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
>> Author: stoklund
>> Date: Thu Apr 19 11:46:26 2012
>> New Revision: 155136
>> 
>> These transformations are deferred:
>> 
>>  (X >>? C) << C   --> X & (-1 << C)  (When X >> C has multiple uses)
>>  (X >>? C1) << C2 --> X << (C2-C1) & (-1 << C2)   (When C2 > C1)
>>  (X >>? C1) << C2 --> X >>? (C1-C2) & (-1 << C2)  (When C1 > C2)
>> 
>> If the form on the left is better, please make sure that we turn the code on the right into the code on the left.
> 
> I don't think we should do that.
> 
> InstCombine doesn't actually have a way of knowing which expression is better, so it will just leave them alone so it at least doesn't break anything.
> 
> The major goal of instcombine is to pick one of two equivalent code constructs, so that the backend and other IR-level optimizers can detect one pattern instead of both. Please make sure that we pick one.

I really don't think the world is that simple.

I could write a pattern to detect shift+mask with certain constants and turn it into chained shifts, but that doesn't mean you will converge to some canonical form for these expressions. Other shift/mask transformations are absent, and I am not convinced that adding them would help anything.

This all gets a lot more complicated when considering that we are optimizing DAGs, and not the trivial expression trees the test cases have.

> SCEV is unusual in that it prefers arithmetic operations to bitwise operations; usually bitwise operations are easier to reason about because they don't carry. There is already logic in SCEV to detect an "or" which is actually an "add", and I think the right fix is to extend that. The premise is that it's easier to find an "or" which is an "add" than it is to find an "add" that is an "or". I'd be interested in hearing if you disagree with that?

Basically, I wouldn't know how to do it. If you start with two expressions:

  Y and Y << 1

Depending on the form Y takes, the canonical form for those two expressions could end up looking completely different. I don't know how scalar evolution could recover from that? The test cases I added are some very simple examples of the problem, where Y = X >> C. More complicated expressions for Y just makes the problem worse.

I think we have to accept that "don't obfuscate existing patterns" should also be a goal of InstCombine.

/jakob

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120419/66302190/attachment.html>


More information about the llvm-commits mailing list