[GlobalISel] Fix legalizer trying to process a deleted instruction.

In some cases an instruction is deleted from the block during combining,
however it can still exist in the legalizer worklist.

This change modifies the combiner routines to add the given MI to the
dead instruction list rather than trying to remove it from the block
themselves. The responsibility is then on the caller to delete the instruction
from the block and any worklists.

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

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

diff --git a/include/llvm/CodeGen/GlobalISel/LegalizerCombiner.h b/include/llvm/CodeGen/GlobalISel/LegalizerCombiner.h
index 607e86d722668414e64fa67fceb74dfe24189f4a..1d501416b18ff3a3ae68a90bc031df4ed38fa681 100644 (file)
--- a/include/llvm/CodeGen/GlobalISel/LegalizerCombiner.h
+++ b/include/llvm/CodeGen/GlobalISel/LegalizerCombiner.h
@@ -41,9 +41,7 @@ public:
       Builder.setInstr(MI);
       // We get a copy/trunc/extend depending on the sizes
       Builder.buildAnyExtOrTrunc(DstReg, SrcReg);
-      MI.eraseFromParent();
-      if (MRI.use_empty(DefMI->getOperand(0).getReg()))
-        DeadInsts.push_back(DefMI);
+      markInstAndDefDead(MI, *DefMI, DeadInsts);
       return true;
     }
     return false;
@@ -68,9 +66,7 @@ public:
       // We get a copy/trunc/extend depending on the sizes
       auto SrcCopyOrTrunc = Builder.buildAnyExtOrTrunc(DstTy, TruncSrc);
       Builder.buildAnd(DstReg, SrcCopyOrTrunc, MaskCstMIB);
-      MI.eraseFromParent();
-      if (MRI.use_empty(DefMI->getOperand(0).getReg()))
-        DeadInsts.push_back(DefMI);
+      markInstAndDefDead(MI, *DefMI, DeadInsts);
       return true;
     }
     return false;
@@ -97,9 +93,7 @@ public:
       auto ShlMIB = Builder.buildInstr(TargetOpcode::G_SHL, DstTy,
                                        SrcCopyExtOrTrunc, SizeDiffMIB);
       Builder.buildInstr(TargetOpcode::G_ASHR, DstReg, ShlMIB, SizeDiffMIB);
-      MI.eraseFromParent();
-      if (MRI.use_empty(DefMI->getOperand(0).getReg()))
-        DeadInsts.push_back(DefMI);
+      markInstAndDefDead(MI, *DefMI, DeadInsts);
       return true;
     }
     return false;
@@ -175,17 +169,14 @@ public:
                            MergeI->getOperand(Idx + 1).getReg());
     }
 
-    MI.eraseFromParent();
-    if (MRI.use_empty(MergeI->getOperand(0).getReg()))
-      DeadInsts.push_back(MergeI);
+    markInstAndDefDead(MI, *MergeI, DeadInsts);
     return true;
   }
 
   /// Try to combine away MI.
   /// Returns true if it combined away the MI.
-  /// Caller should not rely in MI existing as it may be deleted.
   /// Adds instructions that are dead as a result of the combine
-  // into DeadInsts
+  /// into DeadInsts, which can include MI.
   bool tryCombineInstruction(MachineInstr &MI,
                              SmallVectorImpl<MachineInstr *> &DeadInsts) {
     switch (MI.getOpcode()) {
@@ -201,6 +192,16 @@ public:
       return tryCombineMerges(MI, DeadInsts);
     }
   }
+
+private:
+  /// Mark MI as dead. If a def of one of MI's operands, DefMI, would also be
+  /// dead due to MI being killed, then mark DefMI as dead too.
+  void markInstAndDefDead(MachineInstr &MI, MachineInstr &DefMI,
+                          SmallVectorImpl<MachineInstr *> &DeadInsts) {
+    DeadInsts.push_back(&MI);
+    if (MRI.hasOneUse(DefMI.getOperand(0).getReg()))
+      DeadInsts.push_back(&DefMI);
+  }
 };
 
 } // namespace llvm

diff --git a/test/CodeGen/AArch64/GlobalISel/combine-anyext-crash.mir b/test/CodeGen/AArch64/GlobalISel/combine-anyext-crash.mir
new file mode 100644 (file)
index 0000000..339adf5
--- /dev/null
+++ b/test/CodeGen/AArch64/GlobalISel/combine-anyext-crash.mir
@@ -0,0 +1,42 @@
+# RUN: llc -O0 -run-pass=legalizer -global-isel %s -o - | FileCheck %s
+--- |
+  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64--"
+  
+  define void @test_anyext_crash() {
+  entry:
+    br label %block2
+  
+  block2:
+    %0 = trunc i16 0 to i8
+    %1 = uitofp i8 %0 to double
+    br label %block2
+  }
+  
+
+...
+---
+name:            test_anyext_crash
+alignment:       2
+legalized:       false
+registers:       
+  - { id: 0, class: _, preferred-register: '' }
+  - { id: 1, class: _, preferred-register: '' }
+  - { id: 2, class: _, preferred-register: '' }
+body:             |
+  bb.1:
+   ; Check we don't crash due to trying to legalize a dead instruction.
+   ; CHECK-LABEL: test_anyext_crash
+   ; CHECK-LABEL: bb.1:
+    successors: %bb.2
+  
+    %0(s16) = G_CONSTANT i16 0
+  
+  bb.2:
+    successors: %bb.2
+  
+    %1(s8) = G_TRUNC %0(s16)
+    %2(s64) = G_UITOFP %1(s8)
+    G_BR %bb.2
+
+...