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

[Sanjay Patel via llvm-dev]

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.

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



>
>
> 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/7b66db3c/attachment.html>