[PATCH] D123408: [InstCombine] Limit folding of cast into PHI

[Zaara Syeda via Phabricator via llvm-commits]

syzaara added a comment.

In D123408#3571038 <https://reviews.llvm.org/D123408#3571038>, @spatel wrote:

> In D123408#3570605 <https://reviews.llvm.org/D123408#3570605>, @syzaara wrote:
>
>> @spatel Could you please provide some feedback about this change to InstCombine.
>
> In general, making exceptions to transforms will not solve the general problem. I'm not sure if this is enough, but instcombine seems to miss a narrowing transform like this:
> https://alive2.llvm.org/ce/z/hRy3rE
>
> We do already have a variation of that fold, so it should be a small enhancement. I can take a shot at that.
> A demanded bits solution within instcombine might be better, but I'm not seeing how to make that work in general since we have to create 3 instructions from the pattern.

>From what I understand Instcombine has the narrowing transformation already, but there is an issue with order of instructions visited and which optimization runs first. 
For example looking at the debug output, with the changes in this patch:

  IC: Visiting:   %conv2 = trunc i32 %add to i8
  ICE: EvaluateInDifferentType converting expression type to avoid cast:   %conv2 = trunc i32 %add to i8
  IC: Replacing   %conv2 = trunc i32 %1 to i8
  with   %add = add i8 %a.0, %0

It converts:

  %conv = zext i8 %0 to i32
  %conv1 = zext i8 %a.0 to i32
  %add = add nsw i32 %conv1, %conv
  %conv2 = trunc i32 %add to i8

to:

  %add = add i8 %a.0, %0

Without this patch, it first does the optimization to fold the cast into the phi  when visiting: conv3 = zext i8 %a.0 to i32

  IC: Visiting:   %conv3 = zext i8 %a.0 to i32
    ADD DEFERRED:   %0 = phi i32  
    ADD DEFERRED:   %phi.cast = zext i8 %conv2 to i32
    IC: Replacing   %conv1 = zext i8 %0 to i32
        with   %a.0 = phi i32 [ 0, %entry ], [ %phi.cast, %for.body ]

And we no longer get the narrowing transformation:

  %a.0 = phi i32 [ 0, %entry ], [ %phi.cast, %for.body ]
  %conv = zext i8 %0 to i32
  %add = add nuw nsw i32 %a.0, %conv 
  %inc = add i64 %i.0, 1
  %phi.cast = and i32 %add, 255

I am trying to disable the phi cast optimization so that InstCombine can do that narrowing transformation instead.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123408/new/

https://reviews.llvm.org/D123408