llvm dev - Aug 2016 - X86ISelLowering: Promote 'add nsw' to a wider type

Hi Sanjay,
On 19 Jul 2016, at 18:54, Sanjay Patel <spatel at
rotateright.com<mailto:spatel at rotateright.com>> wrote:

Hi Artur -

I don't think there's any reason to limit the transform to sexts only;
that's just the case that was apparent in
https://llvm.org/bugs/show_bug.cgi?id=20134 , so I limited it to that pattern.

It's probably worth noting that I'm currently fighting through casts of
all kinds in IR (InstCombine) rather than the backend:
What is the current status of this work? Does it make sense to patch the
existing code in X86ISelLowering in order to support looking through zext or the
generic solution will be available soon?

Artur
https://reviews.llvm.org/D22421
https://reviews.llvm.org/D22271
https://reviews.llvm.org/D22477
https://reviews.llvm.org/D20774
https://llvm.org/bugs/show_bug.cgi?id=27925

I'm very interested to see what kinds of patterns you're seeing and want
to optimize. It's possible that IR transforms could eliminate the need for
the backend fixups...or it could make them harder. Are there bug reports for the
cases that you are looking at?


On Tue, Jul 19, 2016 at 9:13 AM, Artur Pilipenko <apilipenko at
azulsystems.com<mailto:apilipenko at azulsystems.com>> wrote:
Hi Sanjay,

Some time ago you implemented a sext(add_nsw(x, C)) --> add(sext(x), C_sext)
transformation in X86ISelLowering
https://reviews.llvm.org/D13757
Is there any reason why this transformation is limited to sexts and doesn’t
support zexts?

Thanks,
Artur


-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160808/7802ca71/attachment.html>

> On Aug 8, 2016, at 8:22 AM, Artur Pilipenko via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hi Sanjay,
>> On 19 Jul 2016, at 18:54, Sanjay Patel <spatel at rotateright.com
<mailto:spatel at rotateright.com>> wrote:
>> 
>> Hi Artur -
>> 
>> I don't think there's any reason to limit the transform to
sexts only; that's just the case that was apparent in
https://llvm.org/bugs/show_bug.cgi?id=20134
<https://llvm.org/bugs/show_bug.cgi?id=20134> , so I limited it to that
pattern.
>> 
>> It's probably worth noting that I'm currently fighting through
casts of all kinds in IR (InstCombine) rather than the backend:
> What is the current status of this work? Does it make sense to patch the
existing code in X86ISelLowering in order to support looking through zext or the
generic solution will be available soon?
As a side thought, this might be something best done in a CodeGenPrepare type
position in the pipeline. The DAG is a little bit late because by that point
you’ve lost some information (global information) and may not be able to
eliminate all zexts/sexts accordingly. But instcombine may be a little bit early
because it’ll hurt targets that prefer it the other way around. You could expand
addressing mode stuff in CodeGenPrepare using knownbits or SCEV, maybe? We do
something vaguely similar out of tree (but with the opposite goal, as our target
only supports 32-bit offsets).

—escha
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160808/b6394d3e/attachment.html>

My current status on InstCombine patches: ongoing...no end in sight. If you
look at some of my recent commits, you'll see that I'm trying to get
general InstCombine transforms to work with vector types too. This is
somehow related to the earlier cast patches that I listed, but I've lost
track of how many levels below the original problem I've fallen. :)

But I don't think we can solve the zext equivalent of the sext transform
done with https://reviews.llvm.org/D13757 in InstCombine, so something else
is needed to handle that case.

As escha notes and Quentin mentioned in D13757, CGP is the likely best
place for this sort of thing, but if there are x86-specific (LEA) changes
that you're trying to get, then extending D13757 to zexts is probably the
least effort.

On Mon, Aug 8, 2016 at 10:22 AM, <escha at apple.com> wrote:
>
> On Aug 8, 2016, at 8:22 AM, Artur Pilipenko via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> Hi Sanjay,
>
> On 19 Jul 2016, at 18:54, Sanjay Patel <spatel at rotateright.com>
wrote:
>
> Hi Artur -
>
> I don't think there's any reason to limit the transform to sexts
only;
> that's just the case that was apparent in https://llvm.org/bugs/show_
> bug.cgi?id=20134 , so I limited it to that pattern.
>
> It's probably worth noting that I'm currently fighting through
casts of
> all kinds in IR (InstCombine) rather than the backend:
>
> What is the current status of this work? Does it make sense to patch the
> existing code in X86ISelLowering in order to support looking through zext
> or the generic solution will be available soon?
>
>
> As a side thought, this might be something best done in a CodeGenPrepare
> type position in the pipeline. The DAG is a little bit late because by that
> point you’ve lost some information (global information) and may not be able
> to eliminate all zexts/sexts accordingly. But instcombine may be a little
> bit early because it’ll hurt targets that prefer it the other way around.
> You could expand addressing mode stuff in CodeGenPrepare using knownbits or
> SCEV, maybe? We do something vaguely similar out of tree (but with the
> opposite goal, as our target only supports 32-bit offsets).
>
> —escha
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160808/822ae22d/attachment.html>