llvm dev - Oct 2012 - [LLVMdev] Redundant Add Operation in Code Generation?

That solves the issue but it seems odd to me that instcombine doesn't take
care of it?

So is this just a setup for the backend? If not, seems like if there is a
possibility that lsr could create these redundant operations, should it not
clean itself up? Or am I mistaken?

On Fri, Oct 19, 2012 at 1:29 PM, Andrew Trick <atrick at apple.com> wrote:
>
> On Oct 17, 2012, at 1:22 PM, Ryan Taylor <ryta1203 at gmail.com>
wrote:
>
> I'm curious why I am seeing this:
>
>   *%uglygep18.sum = add i32 %lsr_iv8, %tmp45*
>   %scevgep19 = getelementptr i8* %parBits_017, i32 %uglygep18_sum
>   %scevgep1920 = bitcast i8* %scevgep19 to i16*
>   %tmp78 = load i16* %scevgep1920, align 2
>  * %uglygep14.sum = add i32 %lsr_iv8, %tmp45*
>   %scevgep15 = getelementptr i8* %extIn_013, i32 %uglygep14_sum
>   %scevgep1516 = bitcast i8* %scevgep15 to i16*
>   %tmp79 = load i16* %scevgep1516, align 2
>   %conv93.i.i = sext i16 %tmp79 to i32
>   *%uglygep.sum = add i32 %lsr_iv8, %tmp45*
>   %scevgep11 = getelementptr i8* %sysBits_010, i32 %uglygep_sum
>
> You can see here that "add i32 %lsr_iv8, %tmp45" is done multiple
times,
> appearing that there are two redundant add operations that are not needed
> yet are generated?
>
>
> That's LSR, as you can see from the variable names ;) It might think
that
> load(Base + Index) is a legal addressing mode for your target. -disable-lsr
> might be the right thing for you anyway.
>
> Incidentally, MachineCSE could clean this up if it doesn't get folded
into
> the address, but like LSR, it tries hard not to increase register pressure.
>
> -Andy
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121019/357562ac/attachment.html>

On Oct 19, 2012, at 2:34 PM, Ryan Taylor <ryta1203 at gmail.com> wrote:
> That solves the issue but it seems odd to me that instcombine doesn't
take care of it?
LSR is part of the backend. It's lowering the IR for a specific target. It
seems to think those redundant operations are good for reducing register
pressure, but doesn't actually have much knowledge about register pressure.
At this point, we won't do any more IR level "cleanup" since that
tends to undo lowering. The Machine IR passes will do some careful cleanup.

-Andy
> So is this just a setup for the backend? If not, seems like if there is a
possibility that lsr could create these redundant operations, should it not
clean itself up? Or am I mistaken?
> 
> On Fri, Oct 19, 2012 at 1:29 PM, Andrew Trick <atrick at apple.com>
wrote:
> 
> On Oct 17, 2012, at 1:22 PM, Ryan Taylor <ryta1203 at gmail.com>
wrote:
> 
>> I'm curious why I am seeing this: 
>> 
>>   %uglygep18.sum = add i32 %lsr_iv8, %tmp45
>>   %scevgep19 = getelementptr i8* %parBits_017, i32 %uglygep18_sum
>>   %scevgep1920 = bitcast i8* %scevgep19 to i16*
>>   %tmp78 = load i16* %scevgep1920, align 2
>>   %uglygep14.sum = add i32 %lsr_iv8, %tmp45
>>   %scevgep15 = getelementptr i8* %extIn_013, i32 %uglygep14_sum
>>   %scevgep1516 = bitcast i8* %scevgep15 to i16*
>>   %tmp79 = load i16* %scevgep1516, align 2
>>   %conv93.i.i = sext i16 %tmp79 to i32
>>   %uglygep.sum = add i32 %lsr_iv8, %tmp45
>>   %scevgep11 = getelementptr i8* %sysBits_010, i32 %uglygep_sum
>> 
>> You can see here that "add i32 %lsr_iv8, %tmp45" is done
multiple times, appearing that there are two redundant add operations that are
not needed yet are generated?
> 
> That's LSR, as you can see from the variable names ;) It might think
that load(Base + Index) is a legal addressing mode for your target. -disable-lsr
might be the right thing for you anyway.
> 
> Incidentally, MachineCSE could clean this up if it doesn't get folded
into the address, but like LSR, it tries hard not to increase register pressure.
> 
> -Andy
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121019/97fb4f28/attachment.html>

On 10/19/2012 4:34 PM, Ryan Taylor wrote:
> That solves the issue but it seems odd to me that instcombine doesn't
> take care of it?
>
> So is this just a setup for the backend? If not, seems like if there is
> a possibility that lsr could create these redundant operations, should
> it not clean itself up? Or am I mistaken?
I believe that LSR uses a "context-free" SCEV expander, which will 
process each fixup separately.  I think that this is where the
"uglygep"
code comes from.  If LSR was to detect redundant expressions, it would 
add complexity to the code.  It would be better to simply run CSE as a 
pre-ISel pass if a cleanup before code generation is necessary. 
Otherwise the MI optimizations should be able to take care of this.

-K

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
hosted by The Linux Foundation

Ok, thanks.

Even still though I would expect -instcombine (run after lsr) would do this
cleanup?

On Fri, Oct 19, 2012 at 2:41 PM, Andrew Trick <atrick at apple.com> wrote:
>
> On Oct 19, 2012, at 2:34 PM, Ryan Taylor <ryta1203 at gmail.com>
wrote:
>
> That solves the issue but it seems odd to me that instcombine doesn't
take
> care of it?
>
>
> LSR is part of the backend. It's lowering the IR for a specific target.
It
> seems to think those redundant operations are good for reducing register
> pressure, but doesn't actually have much knowledge about register
pressure.
> At this point, we won't do any more IR level "cleanup" since
that tends to
> undo lowering. The Machine IR passes will do some careful cleanup.
>
> -Andy
>
> So is this just a setup for the backend? If not, seems like if there is a
> possibility that lsr could create these redundant operations, should it not
> clean itself up? Or am I mistaken?
>
> On Fri, Oct 19, 2012 at 1:29 PM, Andrew Trick <atrick at apple.com>
wrote:
>
>>
>> On Oct 17, 2012, at 1:22 PM, Ryan Taylor <ryta1203 at gmail.com>
wrote:
>>
>> I'm curious why I am seeing this:
>>
>>   *%uglygep18.sum = add i32 %lsr_iv8, %tmp45*
>>   %scevgep19 = getelementptr i8* %parBits_017, i32 %uglygep18_sum
>>   %scevgep1920 = bitcast i8* %scevgep19 to i16*
>>   %tmp78 = load i16* %scevgep1920, align 2
>>  * %uglygep14.sum = add i32 %lsr_iv8, %tmp45*
>>   %scevgep15 = getelementptr i8* %extIn_013, i32 %uglygep14_sum
>>   %scevgep1516 = bitcast i8* %scevgep15 to i16*
>>   %tmp79 = load i16* %scevgep1516, align 2
>>   %conv93.i.i = sext i16 %tmp79 to i32
>>   *%uglygep.sum = add i32 %lsr_iv8, %tmp45*
>>   %scevgep11 = getelementptr i8* %sysBits_010, i32 %uglygep_sum
>>
>> You can see here that "add i32 %lsr_iv8, %tmp45" is done
multiple times,
>> appearing that there are two redundant add operations that are not
needed
>> yet are generated?
>>
>>
>> That's LSR, as you can see from the variable names ;) It might
think that
>> load(Base + Index) is a legal addressing mode for your target.
-disable-lsr
>> might be the right thing for you anyway.
>>
>> Incidentally, MachineCSE could clean this up if it doesn't get
folded
>> into the address, but like LSR, it tries hard not to increase register
>> pressure.
>>
>> -Andy
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121019/e9ad35a4/attachment.html>