[GlobalISel][Legalizer] Look thro copies while combining G_UNMERGE's

[rtereshin]

Summary

G_UNMERGE legalization artifact combine is the only one that doesn't look through COPY's for no apparent reason.

As we're becoming stricter w/ respect to not allowing vregs having LLTs and regclasses assigned both mid-globalisel pipeline, the number of extra copies grows, some of which separate G_UNMERGE's from their corresponding G_MERGE's, becoming a performance concern.

This is related to, but not dependent on https://reviews.llvm.org/D45732 and https://reviews.llvm.org/D45640

[rtereshin]

test/CodeGen/AArch64/GlobalISel/legalize-combines.mir

94

test_combines_5 prior to the patch is an exact copy of test_combines_3, so just changing it in place doesn't reduce the coverage nor it does create tests that test more than one thing.

119

same as test_combines_5, but uses one of the copies in the middle of the COPY-chain to see how look-thro-copies thing would deal with it.

test/CodeGen/AArch64/GlobalISel/legalize-nonpowerof2eltsvec.mir

25

My understanding of the test is that it needs to see how the G_MERGE_VALUES would be legalized as an instruction, not as a legalization artifact. To make sure it doesn't get combined out as a legalization artifact instead, we need to use the merged vector value more reliably than by just inserting a copy between G_MERGE_VALUES and G_UNMERGE_VALUES

[aditya_nandakumar]

LGTM. Thanks

[rtereshin]

In D45644#1075661, @aditya_nandakumar wrote:

LGTM. Thanks

Hi Aditya,

Thanks for looking into this!

Roman

include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h

public:
 
   bool tryCombineMerges(MachineInstr &MI,
                         SmallVectorImpl<MachineInstr *> &DeadInsts) {
 
     if (MI.getOpcode() != TargetOpcode::G_UNMERGE_VALUES)
       return false;
 
     unsigned NumDefs = MI.getNumOperands() - 1;
-    unsigned SrcReg = MI.getOperand(NumDefs).getReg();
-    MachineInstr *MergeI = MRI.getVRegDef(SrcReg);
-    if (!MergeI || (MergeI->getOpcode() != TargetOpcode::G_MERGE_VALUES))
+    MachineInstr *MergeI = getOpcodeDef(TargetOpcode::G_MERGE_VALUES,
+                                        MI.getOperand(NumDefs).getReg(), MRI);
+    if (!MergeI)
       return false;
 
     const unsigned NumMergeRegs = MergeI->getNumOperands() - 1;
 
     if (NumMergeRegs < NumDefs) {
       if (NumDefs % NumMergeRegs != 0)
         return false;
 
void markInstAndDefDead(MachineInstr &MI, MachineInstr &DefMI,
     // %1(s1) = G_TRUNC %0(s32)
     // %2(s1) = COPY %1(s1)
     // %3(s1) = COPY %2(s1)
     // %4(s32) = G_ANYEXT %3(s1)
     // In this case, we would have replaced %4 with a copy of %0,
     // and as a result, %3, %2, %1 are dead.
     MachineInstr *PrevMI = &MI;
     while (PrevMI != &DefMI) {
-      // If we're dealing with G_UNMERGE_VALUES, tryCombineMerges doesn't really try
-      // to fold copies in between and we can ignore them here.
-      if (PrevMI->getOpcode() == TargetOpcode::G_UNMERGE_VALUES)
-        break;
-      unsigned PrevRegSrc = PrevMI->getOperand(1).getReg();
+      unsigned PrevRegSrc =
+          PrevMI->getOperand(PrevMI->getNumOperands() - 1).getReg();
       MachineInstr *TmpDef = MRI.getVRegDef(PrevRegSrc);
       if (MRI.hasOneUse(PrevRegSrc)) {
         if (TmpDef != &DefMI) {
           assert(TmpDef->getOpcode() == TargetOpcode::COPY &&
                  "Expecting copy here");
           DeadInsts.push_back(TmpDef);
         }
       } else
         break;
       PrevMI = TmpDef;
     }
-    if ((PrevMI == &DefMI ||
-         DefMI.getOpcode() == TargetOpcode::G_MERGE_VALUES) &&
-        MRI.hasOneUse(DefMI.getOperand(0).getReg()))
+    if (PrevMI == &DefMI && MRI.hasOneUse(DefMI.getOperand(0).getReg()))
       DeadInsts.push_back(&DefMI);
   }
 
   /// Checks if the target legalizer info has specified anything about the
   /// instruction, or if unsupported.
   bool isInstUnsupported(const LegalityQuery &Query) const {
     using namespace LegalizeActions;
     auto Step = LI.getAction(Query);
     return Step.Action == Unsupported || Step.Action == NotFound;
   }
 };
 
 } // namespace llvm

test/CodeGen/AArch64/GlobalISel/legalize-combines.mir

 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
 # RUN: llc -O0 -run-pass=legalizer -global-isel -global-isel-abort=1 %s -o - | FileCheck %s
 
 --- |
   target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
   target triple = "aarch64--"
   define void @test_combines_2() { ret void }
   define void @test_combines_3() { ret void }
   define void @test_combines_4() { ret void }
   define void @test_combines_5() { ret void }
+  define void @test_combines_6() { ret void }
 ...
 
 ---
 name:            test_combines_2
 body: |
   bb.0:
     liveins: $w0
 
bb.0:
     ; CHECK-LABEL: name: test_combines_5
     ; CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY $w0
     ; CHECK: [[ADD:%[0-9]+]]:_(s32) = G_ADD [[COPY]], [[COPY]]
     ; CHECK: [[ADD1:%[0-9]+]]:_(s32) = G_ADD [[COPY]], [[ADD]]
     %0:_(s32) = COPY $w0
 
     %1:_(s32) = G_ADD %0, %0
     %2:_(s64) = G_MERGE_VALUES %0, %1
-    %3:_(s32), %4:_(s32) = G_UNMERGE_VALUES %2
+    %6:_(s64) = COPY %2
+    %7:_(s64) = COPY %6
+    %8:_(s64) = COPY %7
+    %3:_(s32), %4:_(s32) = G_UNMERGE_VALUES %8
+    %5:_(s32) = G_ADD %3, %4
+    $w0 = COPY %5
+...
+
+---
+name:            test_combines_6
+body: |
+  bb.0:
+    liveins: $w0
+
+    ; CHECK-LABEL: name: test_combines_6
+    ; CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+    ; CHECK: [[ADD:%[0-9]+]]:_(s32) = G_ADD [[COPY]], [[COPY]]
+    ; CHECK: [[MV:%[0-9]+]]:_(s64) = G_MERGE_VALUES [[COPY]](s32), [[ADD]](s32)
+    ; CHECK: [[COPY1:%[0-9]+]]:_(s64) = COPY [[MV]](s64)
+    ; CHECK: [[COPY2:%[0-9]+]]:_(s64) = COPY [[COPY1]](s64)
+    ; CHECK: [[ADD1:%[0-9]+]]:_(s32) = G_ADD [[COPY]], [[ADD]]
+    ; CHECK: $w0 = COPY [[ADD1]](s32)
+    ; CHECK: $x0 = COPY [[COPY2]](s64)
+    %0:_(s32) = COPY $w0
+
+    %1:_(s32) = G_ADD %0, %0
+    %2:_(s64) = G_MERGE_VALUES %0, %1
+    %6:_(s64) = COPY %2
+    %7:_(s64) = COPY %6
+    %8:_(s64) = COPY %7
+    %3:_(s32), %4:_(s32) = G_UNMERGE_VALUES %8
     %5:_(s32) = G_ADD %3, %4
     $w0 = COPY %5
+    $x0 = COPY %7
 ...

test/CodeGen/AArch64/GlobalISel/legalize-nonpowerof2eltsvec.mir

registers:
   - { id: 3, class: _ }
   - { id: 4, class: _ }
   - { id: 5, class: _ }
 body: |
   bb.0:
     liveins: $w0
     ; CHECK-LABEL: name: test_legalize_merge_v3s64
     ; CHECK: [[COPY:%[0-9]+]]:_(s64) = COPY $x0
     ; CHECK: [[MV:%[0-9]+]]:_(<3 x s64>) = G_MERGE_VALUES [[COPY]](s64), [[COPY]](s64), [[COPY]](s64)
-    ; CHECK: [[COPY1:%[0-9]+]]:_(<3 x s64>) = COPY [[MV]](<3 x s64>)
-    ; CHECK: [[UV:%[0-9]+]]:_(s64), [[UV1:%[0-9]+]]:_(s64), [[UV2:%[0-9]+]]:_(s64) = G_UNMERGE_VALUES [[COPY1]](<3 x s64>)
-    ; CHECK: $x0 = COPY [[UV]](s64)
+    ; CHECK: $x0 = COPY [[COPY]](s64)
+    ; CHECK: $noreg = PATCHABLE_RET [[MV]](<3 x s64>)
     %0(s64) = COPY $x0
     %1(<3 x s64>) = G_MERGE_VALUES %0(s64), %0(s64), %0(s64)
-    %2(<3 x s64>) = COPY %1(<3 x s64>)
-    %3(s64), %4(s64), %5(s64) = G_UNMERGE_VALUES %2(<3 x s64>)
+    %2(s64), %3(s64), %4(s64) = G_UNMERGE_VALUES %1(<3 x s64>)
     $x0 = COPY %3(s64)
+    $noreg = PATCHABLE_RET %1(<3 x s64>)
 ...