]> granicus.if.org Git - llvm/commitdiff
[GlobalISel] Fix legalizer artifact combiner from crashing with invalid dead instruct...
authorAmara Emerson <aemerson@apple.com>
Wed, 27 Mar 2019 17:47:42 +0000 (17:47 +0000)
committerAmara Emerson <aemerson@apple.com>
Wed, 27 Mar 2019 17:47:42 +0000 (17:47 +0000)
The artifact combiners push instructions which have been marked for deletion
onto an list for the legalizer to deal with on return. However, for trunc(ext)
combines the combiner routine recursively calls itself. When it does this the
dead instructions list may not be empty, and the other combiners don't expect
to be dealing with essentially invalid MIR (multiple vreg defs etc).

This change fixes it by ensuring that the dead instructions are processed on
entry into tryCombineInstruction.

As a result, this fix exposed a few places in tests where G_TRUNC instructions
were not being deleted even though they were dead.

Differential Revision: https://reviews.llvm.org/D59892

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@357101 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
lib/CodeGen/GlobalISel/Legalizer.cpp
test/CodeGen/AArch64/GlobalISel/legalize-phi.mir
test/CodeGen/AArch64/GlobalISel/legalizer-combiner-zext-trunc-crash.mir [new file with mode: 0644]
test/CodeGen/AMDGPU/GlobalISel/legalize-icmp.mir
test/CodeGen/AMDGPU/GlobalISel/legalize-unmerge-values.mir
test/CodeGen/Mips/GlobalISel/legalizer/sub.mir

index e7680e1bc126c10655df2944ed4c428b025ea118..d68704ee623ca63dca406b455551f84e3770d153 100644 (file)
@@ -317,7 +317,13 @@ public:
   /// Adds instructions that are dead as a result of the combine
   /// into DeadInsts, which can include MI.
   bool tryCombineInstruction(MachineInstr &MI,
-                             SmallVectorImpl<MachineInstr *> &DeadInsts) {
+                             SmallVectorImpl<MachineInstr *> &DeadInsts,
+                             GISelObserverWrapper &WrapperObserver) {
+    // This might be a recursive call, and we might have DeadInsts already
+    // populated. To avoid bad things happening later with multiple vreg defs
+    // etc, process the dead instructions now if any.
+    if (!DeadInsts.empty())
+      deleteMarkedDeadInsts(DeadInsts, WrapperObserver);
     switch (MI.getOpcode()) {
     default:
       return false;
@@ -334,7 +340,7 @@ public:
     case TargetOpcode::G_TRUNC: {
       bool Changed = false;
       for (auto &Use : MRI.use_instructions(MI.getOperand(0).getReg()))
-        Changed |= tryCombineInstruction(Use, DeadInsts);
+        Changed |= tryCombineInstruction(Use, DeadInsts, WrapperObserver);
       return Changed;
     }
     }
@@ -395,6 +401,22 @@ private:
       DeadInsts.push_back(&DefMI);
   }
 
+  /// Erase the dead instructions in the list and call the observer hooks.
+  /// Normally the Legalizer will deal with erasing instructions that have been
+  /// marked dead. However, for the trunc(ext(x)) cases we can end up trying to
+  /// process instructions which have been marked dead, but otherwise break the
+  /// MIR by introducing multiple vreg defs. For those cases, allow the combines
+  /// to explicitly delete the instructions before we run into trouble.
+  void deleteMarkedDeadInsts(SmallVectorImpl<MachineInstr *> &DeadInsts,
+                             GISelObserverWrapper &WrapperObserver) {
+    for (auto *DeadMI : DeadInsts) {
+      LLVM_DEBUG(dbgs() << *DeadMI << "Is dead, eagerly deleting\n");
+      WrapperObserver.erasingInstr(*DeadMI);
+      DeadMI->eraseFromParentAndMarkDBGValuesForRemoval();
+    }
+    DeadInsts.clear();
+  }
+
   /// Checks if the target legalizer info has specified anything about the
   /// instruction, or if unsupported.
   bool isInstUnsupported(const LegalityQuery &Query) const {
index bd486d7dd1f187b9845483407b5d2d68ca129244..4db86761cdb256928b15d7654887d0e641257107 100644 (file)
@@ -225,7 +225,8 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
         continue;
       }
       SmallVector<MachineInstr *, 4> DeadInstructions;
-      if (ArtCombiner.tryCombineInstruction(MI, DeadInstructions)) {
+      if (ArtCombiner.tryCombineInstruction(MI, DeadInstructions,
+                                            WrapperObserver)) {
         for (auto *DeadMI : DeadInstructions) {
           LLVM_DEBUG(dbgs() << *DeadMI << "Is dead\n");
           RemoveDeadInstFromLists(DeadMI);
index 484224f842c0089d64d9cc4f799d41f15fac909a..6ab49ff66482f144d4bed9d05e627cfec2eaa734 100644 (file)
@@ -296,18 +296,17 @@ body:             |
   ; CHECK:   [[TRUNC:%[0-9]+]]:_(s16) = G_TRUNC [[C1]](s32)
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
-  ; CHECK:   [[PHI:%[0-9]+]]:_(s16) = G_PHI [[TRUNC]](s16), %bb.0, [[TRUNC3:%[0-9]+]](s16), %bb.1
+  ; CHECK:   [[PHI:%[0-9]+]]:_(s16) = G_PHI [[TRUNC]](s16), %bb.0, [[TRUNC2:%[0-9]+]](s16), %bb.1
   ; CHECK:   [[ANYEXT:%[0-9]+]]:_(s32) = G_ANYEXT [[PHI]](s16)
   ; CHECK:   [[COPY1:%[0-9]+]]:_(s32) = COPY [[C]](s32)
   ; CHECK:   [[ADD:%[0-9]+]]:_(s32) = G_ADD [[ANYEXT]], [[COPY1]]
-  ; CHECK:   [[TRUNC1:%[0-9]+]]:_(s8) = G_TRUNC [[ADD]](s32)
   ; CHECK:   [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 255
   ; CHECK:   [[COPY2:%[0-9]+]]:_(s32) = COPY [[ADD]](s32)
   ; CHECK:   [[AND:%[0-9]+]]:_(s32) = G_AND [[COPY2]], [[C2]]
   ; CHECK:   [[ICMP:%[0-9]+]]:_(s32) = G_ICMP intpred(ugt), [[AND]](s32), [[COPY]]
-  ; CHECK:   [[TRUNC2:%[0-9]+]]:_(s1) = G_TRUNC [[ICMP]](s32)
-  ; CHECK:   [[TRUNC3]]:_(s16) = G_TRUNC [[ADD]](s32)
-  ; CHECK:   G_BRCOND [[TRUNC2]](s1), %bb.1
+  ; CHECK:   [[TRUNC1:%[0-9]+]]:_(s1) = G_TRUNC [[ICMP]](s32)
+  ; CHECK:   [[TRUNC2]]:_(s16) = G_TRUNC [[ADD]](s32)
+  ; CHECK:   G_BRCOND [[TRUNC1]](s1), %bb.1
   ; CHECK: bb.2:
   ; CHECK:   [[C3:%[0-9]+]]:_(s32) = G_CONSTANT i32 255
   ; CHECK:   [[COPY3:%[0-9]+]]:_(s32) = COPY [[ADD]](s32)
@@ -364,14 +363,13 @@ body:             |
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
   ; CHECK:   [[PHI:%[0-9]+]]:_(s16) = G_PHI [[TRUNC]](s16), %bb.0, [[COPY1:%[0-9]+]](s16), %bb.1
-  ; CHECK:   [[TRUNC1:%[0-9]+]]:_(s8) = G_TRUNC [[PHI]](s16)
   ; CHECK:   [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 255
   ; CHECK:   [[ANYEXT:%[0-9]+]]:_(s32) = G_ANYEXT [[PHI]](s16)
   ; CHECK:   [[AND:%[0-9]+]]:_(s32) = G_AND [[ANYEXT]], [[C1]]
   ; CHECK:   [[ICMP:%[0-9]+]]:_(s32) = G_ICMP intpred(ugt), [[AND]](s32), [[COPY]]
-  ; CHECK:   [[TRUNC2:%[0-9]+]]:_(s1) = G_TRUNC [[ICMP]](s32)
+  ; CHECK:   [[TRUNC1:%[0-9]+]]:_(s1) = G_TRUNC [[ICMP]](s32)
   ; CHECK:   [[COPY1]]:_(s16) = COPY [[PHI]](s16)
-  ; CHECK:   G_BRCOND [[TRUNC2]](s1), %bb.1
+  ; CHECK:   G_BRCOND [[TRUNC1]](s1), %bb.1
   ; CHECK: bb.2:
   ; CHECK:   $w0 = COPY [[AND]](s32)
   ; CHECK:   RET_ReallyLR implicit $w0
@@ -547,17 +545,16 @@ body:             |
   ; CHECK:   G_BR %bb.2
   ; CHECK: bb.1:
   ; CHECK:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
-  ; CHECK:   [[PHI:%[0-9]+]]:_(s16) = G_PHI [[TRUNC2]](s16), %bb.0, [[TRUNC5:%[0-9]+]](s16), %bb.1
-  ; CHECK:   [[TRUNC3:%[0-9]+]]:_(s8) = G_TRUNC [[PHI]](s16)
+  ; CHECK:   [[PHI:%[0-9]+]]:_(s16) = G_PHI [[TRUNC2]](s16), %bb.0, [[TRUNC4:%[0-9]+]](s16), %bb.1
   ; CHECK:   [[C5:%[0-9]+]]:_(s32) = G_CONSTANT i32 255
   ; CHECK:   [[ANYEXT:%[0-9]+]]:_(s32) = G_ANYEXT [[PHI]](s16)
   ; CHECK:   [[AND:%[0-9]+]]:_(s32) = G_AND [[ANYEXT]], [[C5]]
   ; CHECK:   [[ADD1:%[0-9]+]]:_(s32) = G_ADD [[AND]], [[C2]]
   ; CHECK:   [[ICMP1:%[0-9]+]]:_(s32) = G_ICMP intpred(ugt), [[ADD1]](s32), [[C3]]
-  ; CHECK:   [[TRUNC4:%[0-9]+]]:_(s1) = G_TRUNC [[ICMP1]](s32)
+  ; CHECK:   [[TRUNC3:%[0-9]+]]:_(s1) = G_TRUNC [[ICMP1]](s32)
   ; CHECK:   [[COPY2:%[0-9]+]]:_(s16) = COPY [[PHI]](s16)
-  ; CHECK:   [[TRUNC5]]:_(s16) = G_TRUNC [[C4]](s32)
-  ; CHECK:   G_BRCOND [[TRUNC4]](s1), %bb.2
+  ; CHECK:   [[TRUNC4]]:_(s16) = G_TRUNC [[C4]](s32)
+  ; CHECK:   G_BRCOND [[TRUNC3]](s1), %bb.2
   ; CHECK:   G_BR %bb.1
   ; CHECK: bb.2:
   ; CHECK:   [[PHI1:%[0-9]+]]:_(s16) = G_PHI [[COPY2]](s16), %bb.1, [[TRUNC1]](s16), %bb.0
diff --git a/test/CodeGen/AArch64/GlobalISel/legalizer-combiner-zext-trunc-crash.mir b/test/CodeGen/AArch64/GlobalISel/legalizer-combiner-zext-trunc-crash.mir
new file mode 100644 (file)
index 0000000..391170c
--- /dev/null
@@ -0,0 +1,72 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple aarch64 -run-pass=legalizer -verify-machineinstrs %s -o - | FileCheck %s
+
+# This test checks we don't crash when doing zext(trunc) legalizer combines.
+---
+name:            zext_trunc_dead_inst_crash
+alignment:       2
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: zext_trunc_dead_inst_crash
+  ; CHECK: bb.0:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 46
+  ; CHECK:   [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 -33
+  ; CHECK:   [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 -65
+  ; CHECK:   [[C3:%[0-9]+]]:_(s32) = G_CONSTANT i32 26
+  ; CHECK:   [[DEF:%[0-9]+]]:_(s16) = G_IMPLICIT_DEF
+  ; CHECK: bb.1:
+  ; CHECK:   successors: %bb.2(0x80000000)
+  ; CHECK:   [[PHI:%[0-9]+]]:_(s16) = G_PHI %33(s16), %bb.2, [[DEF]](s16), %bb.0
+  ; CHECK:   [[C4:%[0-9]+]]:_(s32) = G_CONSTANT i32 255
+  ; CHECK:   [[ANYEXT:%[0-9]+]]:_(s32) = G_ANYEXT [[PHI]](s16)
+  ; CHECK:   [[AND:%[0-9]+]]:_(s32) = G_AND [[ANYEXT]], [[C4]]
+  ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY [[C]](s32)
+  ; CHECK:   [[AND1:%[0-9]+]]:_(s32) = G_AND [[COPY]], [[C4]]
+  ; CHECK:   [[ICMP:%[0-9]+]]:_(s32) = G_ICMP intpred(eq), [[AND]](s32), [[AND1]]
+  ; CHECK:   [[COPY1:%[0-9]+]]:_(s32) = COPY [[ICMP]](s32)
+  ; CHECK:   [[DEF1:%[0-9]+]]:_(s32) = G_IMPLICIT_DEF
+  ; CHECK:   [[OR:%[0-9]+]]:_(s32) = G_OR [[COPY1]], [[DEF1]]
+  ; CHECK:   [[COPY2:%[0-9]+]]:_(s32) = COPY [[C1]](s32)
+  ; CHECK:   [[AND2:%[0-9]+]]:_(s32) = G_AND [[ANYEXT]], [[COPY2]]
+  ; CHECK:   [[COPY3:%[0-9]+]]:_(s32) = COPY [[AND2]](s32)
+  ; CHECK:   [[COPY4:%[0-9]+]]:_(s32) = COPY [[C2]](s32)
+  ; CHECK:   [[ADD:%[0-9]+]]:_(s32) = G_ADD [[COPY3]], [[COPY4]]
+  ; CHECK:   [[COPY5:%[0-9]+]]:_(s32) = COPY [[ADD]](s32)
+  ; CHECK:   [[AND3:%[0-9]+]]:_(s32) = G_AND [[COPY5]], [[C4]]
+  ; CHECK:   [[COPY6:%[0-9]+]]:_(s32) = COPY [[C3]](s32)
+  ; CHECK:   [[AND4:%[0-9]+]]:_(s32) = G_AND [[COPY6]], [[C4]]
+  ; CHECK:   [[ICMP1:%[0-9]+]]:_(s32) = G_ICMP intpred(ult), [[AND3]](s32), [[AND4]]
+  ; CHECK:   [[COPY7:%[0-9]+]]:_(s32) = COPY [[ICMP1]](s32)
+  ; CHECK:   [[COPY8:%[0-9]+]]:_(s32) = COPY [[OR]](s32)
+  ; CHECK:   [[OR1:%[0-9]+]]:_(s32) = G_OR [[COPY7]], [[COPY8]]
+  ; CHECK:   [[TRUNC:%[0-9]+]]:_(s1) = G_TRUNC [[OR1]](s32)
+  ; CHECK:   G_BRCOND [[TRUNC]](s1), %bb.2
+  ; CHECK: bb.2:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   [[C5:%[0-9]+]]:_(s32) = G_CONSTANT i32 64
+  ; CHECK:   [[TRUNC1:%[0-9]+]]:_(s16) = G_TRUNC [[C5]](s32)
+  ; CHECK:   G_BR %bb.1
+  bb.1:
+    %1:_(s8) = G_CONSTANT i8 46
+    %3:_(s1) = G_IMPLICIT_DEF
+    %5:_(s8) = G_CONSTANT i8 -33
+    %7:_(s8) = G_CONSTANT i8 -65
+    %9:_(s8) = G_CONSTANT i8 26
+    %13:_(s8) = G_IMPLICIT_DEF
+
+  bb.2:
+    %0:_(s8) = G_PHI %12(s8), %bb.4, %13(s8), %bb.1
+    %2:_(s1) = G_ICMP intpred(eq), %0(s8), %1
+    %4:_(s1) = G_OR %2, %3
+    %6:_(s8) = G_AND %0, %5
+    %8:_(s8) = G_ADD %6, %7
+    %10:_(s1) = G_ICMP intpred(ult), %8(s8), %9
+    %11:_(s1) = G_OR %10, %4
+    G_BRCOND %11(s1), %bb.4
+
+  bb.4:
+    %12:_(s8) = G_CONSTANT i8 64
+    G_BR %bb.2
+
+...
index ca6bbea9095a870e83b8a49fc08c85eafc21c4e6..bfd7c8e12f7f4df088b9ed0af78c858b4b1a1140 100644 (file)
@@ -44,7 +44,6 @@ body: |
     liveins: $vgpr0
     ; CHECK-LABEL: name: test_icmp_s16
     ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
-    ; CHECK: [[TRUNC:%[0-9]+]]:_(s16) = G_TRUNC [[C]](s32)
     ; CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
     ; CHECK: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 65535
     ; CHECK: [[COPY1:%[0-9]+]]:_(s32) = COPY [[C]](s32)
@@ -74,7 +73,6 @@ body: |
     liveins: $vgpr0
     ; CHECK-LABEL: name: test_icmp_s8
     ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
-    ; CHECK: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[C]](s32)
     ; CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
     ; CHECK: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 255
     ; CHECK: [[COPY1:%[0-9]+]]:_(s32) = COPY [[C]](s32)
index f7adb875301a4625bbe9afedf5ccf8442649181e..c2a2bcb5bb7302c598eaebf02ada92fb6125f9ad 100644 (file)
@@ -211,17 +211,16 @@ body: |
     ; CHECK: [[AND1:%[0-9]+]]:_(s64) = G_AND [[ANYEXT1]], [[C2]]
     ; CHECK: [[COPY1:%[0-9]+]]:_(s64) = COPY [[SHL]](s64)
     ; CHECK: [[OR:%[0-9]+]]:_(s64) = G_OR [[AND1]], [[COPY1]]
-    ; CHECK: [[TRUNC2:%[0-9]+]]:_(s48) = G_TRUNC [[OR]](s64)
     ; CHECK: [[C3:%[0-9]+]]:_(s64) = G_CONSTANT i64 30
-    ; CHECK: [[TRUNC3:%[0-9]+]]:_(s48) = G_TRUNC [[C3]](s64)
-    ; CHECK: [[TRUNC4:%[0-9]+]]:_(s32) = G_TRUNC [[TRUNC3]](s48)
+    ; CHECK: [[TRUNC2:%[0-9]+]]:_(s48) = G_TRUNC [[C3]](s64)
+    ; CHECK: [[TRUNC3:%[0-9]+]]:_(s32) = G_TRUNC [[TRUNC2]](s48)
     ; CHECK: [[COPY2:%[0-9]+]]:_(s64) = COPY [[OR]](s64)
-    ; CHECK: [[SHL1:%[0-9]+]]:_(s64) = G_SHL [[COPY2]], [[TRUNC4]](s32)
+    ; CHECK: [[SHL1:%[0-9]+]]:_(s64) = G_SHL [[COPY2]], [[TRUNC3]](s32)
     ; CHECK: [[COPY3:%[0-9]+]]:_(s64) = COPY [[OR]](s64)
     ; CHECK: [[COPY4:%[0-9]+]]:_(s64) = COPY [[SHL1]](s64)
     ; CHECK: [[OR1:%[0-9]+]]:_(s64) = G_OR [[COPY3]], [[COPY4]]
-    ; CHECK: [[TRUNC5:%[0-9]+]]:_(s48) = G_TRUNC [[OR1]](s64)
-    ; CHECK: [[UV:%[0-9]+]]:_(s16), [[UV1:%[0-9]+]]:_(s16), [[UV2:%[0-9]+]]:_(s16) = G_UNMERGE_VALUES [[TRUNC5]](s48)
+    ; CHECK: [[TRUNC4:%[0-9]+]]:_(s48) = G_TRUNC [[OR1]](s64)
+    ; CHECK: [[UV:%[0-9]+]]:_(s16), [[UV1:%[0-9]+]]:_(s16), [[UV2:%[0-9]+]]:_(s16) = G_UNMERGE_VALUES [[TRUNC4]](s48)
     ; CHECK: [[ANYEXT2:%[0-9]+]]:_(s32) = G_ANYEXT [[UV]](s16)
     ; CHECK: [[ANYEXT3:%[0-9]+]]:_(s32) = G_ANYEXT [[UV1]](s16)
     ; CHECK: [[ANYEXT4:%[0-9]+]]:_(s32) = G_ANYEXT [[UV2]](s16)
index 9f84e4d05015b66b5af7c73bbe66d468dfc5d8c0..3b80ca090ed2f6dbac1210a37a6045147043b9cf 100644 (file)
@@ -279,7 +279,6 @@ body:             |
     ; MIPS32: [[LOAD3:%[0-9]+]]:_(s32) = G_LOAD [[FRAME_INDEX3]](p0) :: (load 4 from %fixed-stack.3)
     ; MIPS32: [[SUB:%[0-9]+]]:_(s32) = G_SUB [[LOAD]], [[COPY]]
     ; MIPS32: [[ICMP:%[0-9]+]]:_(s32) = G_ICMP intpred(ult), [[LOAD]](s32), [[COPY]]
-    ; MIPS32: [[TRUNC:%[0-9]+]]:_(s1) = G_TRUNC [[ICMP]](s32)
     ; MIPS32: [[SUB1:%[0-9]+]]:_(s32) = G_SUB [[LOAD1]], [[COPY1]]
     ; MIPS32: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 1
     ; MIPS32: [[COPY4:%[0-9]+]]:_(s32) = COPY [[ICMP]](s32)
@@ -293,7 +292,6 @@ body:             |
     ; MIPS32: [[COPY7:%[0-9]+]]:_(s32) = COPY [[ICMP1]](s32)
     ; MIPS32: [[AND1:%[0-9]+]]:_(s32) = G_AND [[COPY7]], [[C1]]
     ; MIPS32: [[SELECT:%[0-9]+]]:_(s32) = G_SELECT [[AND1]](s32), [[COPY5]], [[COPY6]]
-    ; MIPS32: [[TRUNC1:%[0-9]+]]:_(s1) = G_TRUNC [[SELECT]](s32)
     ; MIPS32: [[SUB3:%[0-9]+]]:_(s32) = G_SUB [[LOAD2]], [[COPY2]]
     ; MIPS32: [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 1
     ; MIPS32: [[COPY8:%[0-9]+]]:_(s32) = COPY [[SELECT]](s32)