[PATCH] D62338: [globalisel][legalizer] Combine G_TRUNC+G_MERGE_VALUES in artifact combiner

Fri May 24 06:38:36 PDT 2019

Petar.Avramovic added a comment.

Patch alone looks good.
I'm just coming from D61787 <https://reviews.llvm.org/D61787>,  hopefully it shouldn't be to complicated to make them both work together.
Also targets does not have to define any legalization rules for this combine to work which is great.

In D61787 <https://reviews.llvm.org/D61787> I mentioned that it might be easier for artifact combiner if this was handled with adding narrow scalar rule for G_TRUNC(G_UNMERGE+COPY) and allowing combiner to finish with combining  G_UNMERGE/G_MERGE. 
What do you think is better in the sense that we also want to allow legalization of chained artifacts?

Now we have G_SEXT/G_TRUNC combine and G_TRUNC/G_UNMERGE_VALUES, which one should happen first in a sequence like this?

  %2:_(s128) = G_MERGE_VALUES %0:_(s64), %1:_(s64) 
  %3:_(s64) = G_TRUNC %2(s128)
  %4:_(s128) = G_SEXT %3:_(s64)
    . . .   = G_UNMERGE_VALUES %4:_(s128)

================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-unmerge-values.mir:253-257
     ; CHECK: [[MV:%[0-9]+]]:_(s128) = G_MERGE_VALUES [[AND]](s64), [[AND1]](s64)
     ; CHECK: [[C2:%[0-9]+]]:_(s64) = G_CONSTANT i64 15
-    ; CHECK: [[MV1:%[0-9]+]]:_(s128) = G_MERGE_VALUES [[C2]](s64), [[C1]](s64)
-    ; CHECK: [[TRUNC:%[0-9]+]]:_(s32) = G_TRUNC [[MV1]](s128)
+    ; CHECK: [[TRUNC:%[0-9]+]]:_(s32) = G_TRUNC [[C2]](s64)
     ; CHECK: [[C3:%[0-9]+]]:_(s32) = G_CONSTANT i32 64
     ; CHECK: [[UV2:%[0-9]+]]:_(s64), [[UV3:%[0-9]+]]:_(s64) = G_UNMERGE_VALUES [[MV]](s128)
----------------
Uncombined G_UNMERGE/G_MERGE pair.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62338