[PATCH] D37069: [x86] use the IR type of formal args to create assertzext/assertsext and scalar truncate nodes

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D37069#858329, @efriedma wrote:

> Using more aggressive AssertZext/etc. when we can seems obviously good.  I don't see what that has to do with modifying the type of ArgValue, though.


IIUC, you're suggesting the solution that @aaboud was probably making earlier, but I didn't recognize it at the time:

1. Fix the x86 (and other targets*) assertzext generation to use the IR type because that has to be equal or smaller than the register type (that's the first part of the x86 diff in this rev of the patch near line 3000).
2. Don't alter the ArgValue types.
3. Fix the later chunk in SelectionDAGISel::LowerArguments() to not create a redundant assertzext if the target has already created one.

This produces identical x86 test diffs as we see here. We'll still have a trunc-of-trunc for i32 -> i8 -> i1 in the affected x86 cases, but those will get folded by existing logic.
*For in-tree targets, I would need to make a similar fix as the x86 change to PowerPC, Mips, and AMDGPU because I see regression test failures for all of those.

But...
As I was looking at what Mips was doing I discovered:

  performAssertZextCombine()
  // fold (AssertZext (trunc (AssertZext x))) -> (trunc (AssertZext x))
  // if the type of the extension of the innermost AssertZext node is
  // smaller from that of the outermost node, eg:
  // (AssertZext:i32 (trunc:i32 (AssertZext:i64 X, i32)), i8)
  //   -> (trunc:i32 (AssertZext X, i8))

This is similar to what I proposed as a general DAGCombine in https://reviews.llvm.org/D37017. Should we go ahead with that patch and avoid all of the target-specific changes? The upside is that out-of-tree targets likely have the same bug, so if we add a generic combine, everyone gets the improvement. The downside is slightly less efficiency from creating an unnecessary assert node that ends up being removed.


https://reviews.llvm.org/D37069