[PATCH] D66769: [GlobalISel] Import patterns containing SUBREG_TO_REG

[Amara Emerson via Phabricator via llvm-commits]

aemerson added a subscriber: t.p.northover.
aemerson added a comment.

> This is kind of a hit/miss for code size improvements/regressions. E.g. in add-ext.ll, we now get some identity copies. This isn't really anything the importer can handle, since it's caused by a later pass introducing the copy for the sake of correctness.

To clarify this for upstream after we had some private discussion: what's happening here is that the x86 GISel backend previously implemented i16->i32 anyexts with custom C++ selection as INSERT_SUBREG. With this change, we start importing the new SelectionDAG pattern which changed the anyext representation to SUBREG_TO_REG. As a result, the imul instruction which is destructive in it's first source operand, and thus relies on the TwoAddress pass to add copies, now results in an additional copy because TwoAddress commutes the source ops, causing the SUBREG_TO_REG to be the destroyed operand, and therefore we see an extra copy inserted. There isn't really anything GISel can do here it seems to avoid this. There might be the case for identity copies to be eliminated after regalloc though, it's surprising it doesn't already happen.



================
Comment at: llvm/test/CodeGen/X86/GlobalISel/select-ext-x86-64.mir:139
     ; ALL: [[COPY1:%[0-9]+]]:gr8 = COPY [[COPY]].sub_8bit
-    ; ALL: [[SUBREG_TO_REG:%[0-9]+]]:gr64 = SUBREG_TO_REG 0, [[COPY1]], %subreg.sub_8bit
+    ; ALL: [[MOVZX32rr8_:%[0-9]+]]:gr32 = MOVZX32rr8 [[COPY1]]
+    ; ALL: [[SUBREG_TO_REG:%[0-9]+]]:gr64 = SUBREG_TO_REG 0, [[MOVZX32rr8_]], %subreg.sub_32bit
----------------
I've verified that this code is exactly what the x86 SDAG backend wants for the pattern. The original pattern seems to have been introduced by @t.p.northover in r182921. Why an anyext of GR8->i64 would want to use a mov + subreg_to_reg instead of just a subreg_to_reg I don't know.


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

https://reviews.llvm.org/D66769