[PATCH] D45065: [InstCombine] Fix PR17564: don't fold [zs]ext into phi.

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 2 06:48:40 PDT 2018


spatel added a comment.

In https://reviews.llvm.org/D45065#1054274, @lebedev.ri wrote:

> In https://reviews.llvm.org/D45065#1054271, @spatel wrote:
>
> > Also, some of the tests show the benefit of the existing transform - we eliminate an instruction by changing the type of the phi.
>
>
> Uhm, which one specifically? As far as i can see, all tests here end up with the same instructions, just some are moved around.


@zext_of_phi / @sext_of_phi - currently, in those tests we absorb the zext/sext into the phi removing an instruction, right?

> But in `@icmp_div2` we actually loose an instruction, and always return 1.

I think that's an interesting and independent case. It looks like we're losing information because of the order of the folds? Or maybe we're missing an instsimplify for phi with an undef operand?

> Hmm, right, somehow i missed those when looking where to work on https://reviews.llvm.org/D45108

Yes, that code is a mess. Any preliminary cleanup patches to organize and comment it would be appreciated. I've been putting off refactoring decomposeBitTestICmp() to include the cases for ICMP_EQ / ICMP_NE, but this bug may be the needed motivation.

> So, i should close this?

Yes.


Repository:
  rL LLVM

https://reviews.llvm.org/D45065





More information about the llvm-commits mailing list