From: Amara Emerson <aemerson@apple.com>
Date: Fri, 6 Oct 2017 19:24:15 +0000 (+0000)
Subject: [GlobalISel] Fix legalizer trying to process a deleted instruction.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0ef919dcd45b4e7276905017bbece639d3110b96;p=llvm

[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 607e86d7226..1d501416b18 100644
--- 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
index 00000000000..339adf51451
--- /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
+
+...