[PATCH] D33338: [InstCombineCasts] Take in account final size when transforming sext->lshr->trunc patterns

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

I think it's good to solve the miscompile case ASAP, but fundamentally, this block of code is error-prone and incomplete because we're missing earlier folds that would have kept us out of trouble from trying to reason about a 3 instruction sequence (trunc(lshr (sext A), C).

For example, the modified PR33078 pattern in the test here could be canonicalized like this

  Name: smear_hibit
  %sext = sext i3 %x to i16
  %r = lshr i16 %sext, 13
    =>
  %s = ashr i3 %x, 2
  %r = zext i3 %s to i16

Ie, we should lift the shift because narrower ops are easier for value tracking. We might also consider it an optimization / strength-reduction to replace a wide shift with a narrow shift.

If we had this pattern, the zext followed by trunc would get optimized away cleanly using an existing simple fold that's easy to reason about.

In the actual PR33078 problem description case, there is no question that this is a missed fold:

  Name: bool_hibit
  %sext = sext i1 %x to i16
  %hibit = lshr i16 %sext, 15
    =>
  %hibit = zext i1 %x to i16

This should again become a zext/trunc pair for PR33078, and we would never reach the buggy block of code.
I would prefer to solve the bug by fixing the most basic patterns. By doing that, we should be able to simplify - or hopefully remove entirely - this block of code that is looking for longer patterns.


https://reviews.llvm.org/D33338