From 80726a4dd80539f2aa4521012e60e7048db125d1 Mon Sep 17 00:00:00 2001
From: Aditya Nandakumar <aditya_nandakumar@apple.com>
Date: Fri, 7 Apr 2017 21:49:30 +0000
Subject: [PATCH] [GlobalISel]: Fix bug where we can report GISelFailure on
 erased instructions

The original instruction might get legalized and erased and expanded
into intermediate instructions and the intermediate instructions might
fail legalization. This end up in reporting GISelFailure on the erased
instruction.
Instead report GISelFailure on the intermediate instruction which failed
legalization.

Reviewed by: ab

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@299802 91177308-0d34-0410-b5e6-96231b3b80d8
---
 .../llvm/CodeGen/GlobalISel/LegalizerHelper.h |  7 ++--
 lib/CodeGen/GlobalISel/Legalizer.cpp          | 39 ++++++++++++-------
 lib/CodeGen/GlobalISel/LegalizerHelper.cpp    | 25 ------------
 .../gisel-fail-intermediate-legalizer.ll      |  8 ++++
 4 files changed, 38 insertions(+), 41 deletions(-)
 create mode 100644 test/CodeGen/AArch64/GlobalISel/gisel-fail-intermediate-legalizer.ll

diff --git a/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h b/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
index ec8e141893e..8fecafdc08d 100644
--- a/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
+++ b/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
@@ -57,8 +57,6 @@ public:
   /// registers as \p MI.
   LegalizeResult legalizeInstrStep(MachineInstr &MI);
 
-  LegalizeResult legalizeInstr(MachineInstr &MI);
-
   /// Legalize an instruction by emiting a runtime library call instead.
   LegalizeResult libcall(MachineInstr &MI);
 
@@ -85,6 +83,10 @@ public:
   LegalizeResult moreElementsVector(MachineInstr &MI, unsigned TypeIdx,
                                     LLT WideTy);
 
+  /// Expose MIRBuilder so clients can set their own RecordInsertInstruction
+  /// functions
+  MachineIRBuilder MIRBuilder;
+
 private:
 
   /// Helper function to split a wide generic register into bitwise blocks with
@@ -93,7 +95,6 @@ private:
   void extractParts(unsigned Reg, LLT Ty, int NumParts,
                     SmallVectorImpl<unsigned> &Ops);
 
-  MachineIRBuilder MIRBuilder;
   MachineRegisterInfo &MRI;
   const LegalizerInfo &LI;
 };
diff --git a/lib/CodeGen/GlobalISel/Legalizer.cpp b/lib/CodeGen/GlobalISel/Legalizer.cpp
index 849363caf09..657ddb30791 100644
--- a/lib/CodeGen/GlobalISel/Legalizer.cpp
+++ b/lib/CodeGen/GlobalISel/Legalizer.cpp
@@ -171,21 +171,34 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
       // and are assumed to be legal.
       if (!isPreISelGenericOpcode(MI->getOpcode()))
         continue;
-
-      auto Res = Helper.legalizeInstr(*MI);
-
-      // Error out if we couldn't legalize this instruction. We may want to fall
-      // back to DAG ISel instead in the future.
-      if (Res == LegalizerHelper::UnableToLegalize) {
-        reportGISelFailure(MF, TPC, MORE, "gisel-legalize",
-                           "unable to legalize instruction", *MI);
-        return false;
-      }
-
-      Changed |= Res == LegalizerHelper::Legalized;
+      SmallVector<MachineInstr *, 4> WorkList;
+      Helper.MIRBuilder.recordInsertions(
+          [&](MachineInstr *MI) { WorkList.push_back(MI); });
+      WorkList.push_back(&*MI);
+
+      LegalizerHelper::LegalizeResult Res;
+      unsigned Idx = 0;
+      do {
+        Res = Helper.legalizeInstrStep(*WorkList[Idx]);
+        // Error out if we couldn't legalize this instruction. We may want to
+        // fall
+        // back to DAG ISel instead in the future.
+        if (Res == LegalizerHelper::UnableToLegalize) {
+          Helper.MIRBuilder.stopRecordingInsertions();
+          if (Res == LegalizerHelper::UnableToLegalize) {
+            reportGISelFailure(MF, TPC, MORE, "gisel-legalize",
+                               "unable to legalize instruction",
+                               *WorkList[Idx]);
+            return false;
+          }
+        }
+        Changed |= Res == LegalizerHelper::Legalized;
+        ++Idx;
+      } while (Idx < WorkList.size());
+
+      Helper.MIRBuilder.stopRecordingInsertions();
     }
 
-
   MachineRegisterInfo &MRI = MF.getRegInfo();
   const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
   for (auto &MBB : MF) {
diff --git a/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index 17b7e294b6b..afab23d94f0 100644
--- a/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -57,31 +57,6 @@ LegalizerHelper::legalizeInstrStep(MachineInstr &MI) {
   }
 }
 
-LegalizerHelper::LegalizeResult
-LegalizerHelper::legalizeInstr(MachineInstr &MI) {
-  SmallVector<MachineInstr *, 4> WorkList;
-  MIRBuilder.recordInsertions(
-      [&](MachineInstr *MI) { WorkList.push_back(MI); });
-  WorkList.push_back(&MI);
-
-  bool Changed = false;
-  LegalizeResult Res;
-  unsigned Idx = 0;
-  do {
-    Res = legalizeInstrStep(*WorkList[Idx]);
-    if (Res == UnableToLegalize) {
-      MIRBuilder.stopRecordingInsertions();
-      return UnableToLegalize;
-    }
-    Changed |= Res == Legalized;
-    ++Idx;
-  } while (Idx < WorkList.size());
-
-  MIRBuilder.stopRecordingInsertions();
-
-  return Changed ? Legalized : AlreadyLegal;
-}
-
 void LegalizerHelper::extractParts(unsigned Reg, LLT Ty, int NumParts,
                                    SmallVectorImpl<unsigned> &VRegs) {
   for (int i = 0; i < NumParts; ++i)
diff --git a/test/CodeGen/AArch64/GlobalISel/gisel-fail-intermediate-legalizer.ll b/test/CodeGen/AArch64/GlobalISel/gisel-fail-intermediate-legalizer.ll
new file mode 100644
index 00000000000..e333f742e04
--- /dev/null
+++ b/test/CodeGen/AArch64/GlobalISel/gisel-fail-intermediate-legalizer.ll
@@ -0,0 +1,8 @@
+;RUN: llc -mtriple=aarch64-unknown-unknown -o - -global-isel -global-isel-abort=2 %s 2>&1 | FileCheck %s
+; CHECK: fallback
+; CHECK-LABEL: foo
+define i16 @foo(half* %p) {
+  %tmp0 = load half, half* %p
+  %tmp1 = fptoui half %tmp0 to i16
+  ret i16 %tmp1
+}
-- 
2.40.0