[llvm-dev] Folding zext from i1 into PHI nodes with only zwo incoming values.

[Björn Steinbrink via llvm-dev]

2017-01-31 16:59 GMT+01:00 Sanjay Patel <spatel at rotateright.com>:

>
>
> On Tue, Jan 31, 2017 at 3:09 AM, Björn Steinbrink <bsteinbr at gmail.com>
> wrote:
>
>> Hi Sanjay,
>>
>> 2017-01-30 22:22 GMT+01:00 Sanjay Patel <spatel at rotateright.com>:
>>
>>> My minimal patch idea is to ease the restriction in ShouldChangeType
>>> because i1 is special. I tried the patch below, and it works on the
>>> example...and nothing in 'make check' failed. :)
>>>
>>
>> Yeah, that would work for me as well, I just wasn't sure about the
>> implications that has. Simply making FoldPHIArgOpIntoPHI act like
>> FoldPHIArgZextsIntoPHI seemed like the safer option to me, but I wanted
>> feedback on it before creating a PR. Do you want to go ahead with that
>> minimal approach and create a PR yourself?
>>
>
> Your idea probably has the minimal impact while still fixing your problem
> case...although the relationships between FoldPHIArgOpIntoPHI /
> FoldPHIArgZextsIntoPHI / FoldOpIntoPhi are confusing.
>
> I think a patch for ShouldChangeType makes sense on its own (and makes any
> direct changes to the FoldPHI* functions unnecessary for your example).
> I'll try some more experiments with that patch to look for unintended
> consequences.
>

OK, I'll wait for your results then


> When you say "PR", do you mean a bugzilla report or a patch proposal on
> Phab?
>

I meant a patch proposal on Phab, I'm too used to GitHub terminology by
now, sorry.

Cheers,
Björn


>
>
>>
>>
>> Björn
>>
>>
>>> Index: lib/Transforms/InstCombine/InstructionCombining.cpp
>>> ===================================================================
>>> --- lib/Transforms/InstCombine/InstructionCombining.cpp    (revision
>>> 293485)
>>> +++ lib/Transforms/InstCombine/InstructionCombining.cpp    (working
>>> copy)
>>> @@ -88,12 +88,12 @@
>>>
>>>  /// Return true if it is desirable to convert an integer computation
>>> from a
>>>  /// given bit width to a new bit width.
>>> -/// We don't want to convert from a legal to an illegal type for
>>> example or from
>>> -/// a smaller to a larger illegal type.
>>> +/// We don't want to convert from a legal to an illegal type or from a
>>> smaller
>>> +/// to a larger illegal type. Width of '1' is always treated as a legal
>>> type.
>>>  bool InstCombiner::ShouldChangeType(unsigned FromWidth,
>>>                                      unsigned ToWidth) const {
>>> -  bool FromLegal = DL.isLegalInteger(FromWidth);
>>> -  bool ToLegal = DL.isLegalInteger(ToWidth);
>>> +  bool FromLegal = FromWidth == 1 ? true : DL.isLegalInteger(FromWidth);
>>> +  bool ToLegal = ToWidth == 1 ? true : DL.isLegalInteger(ToWidth);
>>>
>>>    // If this is a legal integer from type, and the result would be an
>>> illegal
>>>    // type, don't do the transformation.
>>> @@ -110,7 +110,7 @@
>>>
>>>  /// Return true if it is desirable to convert a computation from 'From'
>>> to 'To'.
>>>  /// We don't want to convert from a legal to an illegal type for
>>> example or from
>>> -/// a smaller to a larger illegal type.
>>> +/// a smaller to a larger illegal type. i1 is always treated as a legal
>>> type.
>>>  bool InstCombiner::ShouldChangeType(Type *From, Type *To) const {
>>>    assert(From->isIntegerTy() && To->isIntegerTy());
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170131/64798dc6/attachment.html>