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

[Amara Emerson via llvm-commits]

Author: aemerson
Date: Fri Oct  6 12:24:15 2017
New Revision: 315092

URL: http://llvm.org/viewvc/llvm-project?rev=315092&view=rev
Log:
[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

Added:
    llvm/trunk/test/CodeGen/AArch64/GlobalISel/combine-anyext-crash.mir
Modified:
    llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerCombiner.h

Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerCombiner.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerCombiner.h?rev=315092&r1=315091&r2=315092&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerCombiner.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerCombiner.h Fri Oct  6 12:24:15 2017
@@ -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

Added: llvm/trunk/test/CodeGen/AArch64/GlobalISel/combine-anyext-crash.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/combine-anyext-crash.mir?rev=315092&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/combine-anyext-crash.mir (added)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/combine-anyext-crash.mir Fri Oct  6 12:24:15 2017
@@ -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
+
+...