[PATCH] D31503: [GlobalISel]: Fix bug where we can report GISelFailure on erased instructions

[Aditya Nandakumar via Phabricator via llvm-commits]

aditya_nandakumar created this revision.
Herald added subscribers: igorb, kristof.beyls, dberris.

The original instruction might get legalized and erased and expanded

  into intermediate insts. An intermediate instruction might fail
  legalization and we would report failure on an erased inst.
  Instead now report failure on the first failing instruction


Repository:
  rL LLVM

https://reviews.llvm.org/D31503

Files:
  include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
  lib/CodeGen/GlobalISel/Legalizer.cpp
  lib/CodeGen/GlobalISel/LegalizerHelper.cpp


Index: lib/CodeGen/GlobalISel/LegalizerHelper.cpp
===================================================================
--- lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -16,7 +16,9 @@
 #include "llvm/CodeGen/GlobalISel/LegalizerHelper.h"
 #include "llvm/CodeGen/GlobalISel/CallLowering.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
+#include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
+#include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetLowering.h"
@@ -28,8 +30,8 @@
 
 using namespace llvm;
 
-LegalizerHelper::LegalizerHelper(MachineFunction &MF)
-    : MRI(MF.getRegInfo()), LI(*MF.getSubtarget().getLegalizerInfo()) {
+LegalizerHelper::LegalizerHelper(MachineFunction &MF, const TargetPassConfig &TPC)
+    : MRI(MF.getRegInfo()), LI(*MF.getSubtarget().getLegalizerInfo()), TPC(TPC) {
   MIRBuilder.setMF(MF);
 }
 
@@ -68,9 +70,16 @@
   LegalizeResult Res;
   unsigned Idx = 0;
   do {
-    Res = legalizeInstrStep(*WorkList[Idx]);
+    MachineInstr *TmpMI = WorkList[Idx];
+    Res = legalizeInstrStep(*TmpMI);
+    // Error out if we couldn't legalize this instruction. We may want to fall
+    // back to DAG ISel instead in the future.
     if (Res == UnableToLegalize) {
       MIRBuilder.stopRecordingInsertions();
+      MachineFunction *MF = TmpMI->getParent()->getParent();
+      MachineOptimizationRemarkEmitter MORE(*MF, /*MBFI=*/nullptr);
+      reportGISelFailure(*MF, TPC, MORE, "gisel-legalize",
+                         "unable to legalize instruction", *TmpMI);
       return UnableToLegalize;
     }
     Changed |= Res == Legalized;
Index: lib/CodeGen/GlobalISel/Legalizer.cpp
===================================================================
--- lib/CodeGen/GlobalISel/Legalizer.cpp
+++ lib/CodeGen/GlobalISel/Legalizer.cpp
@@ -152,7 +152,7 @@
   init(MF);
   const TargetPassConfig &TPC = getAnalysis<TargetPassConfig>();
   MachineOptimizationRemarkEmitter MORE(MF, /*MBFI=*/nullptr);
-  LegalizerHelper Helper(MF);
+  LegalizerHelper Helper(MF, TPC);
 
   // FIXME: an instruction may need more than one pass before it is legal. For
   // example on most architectures <3 x i3> is doubly-illegal. It would
@@ -172,13 +172,10 @@
       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;
       }
 
Index: include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
===================================================================
--- include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
+++ include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
@@ -24,6 +24,7 @@
 #include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/LowLevelType.h"
+#include "llvm/CodeGen/TargetPassConfig.h"
 
 namespace llvm {
 // Forward declarations.
@@ -46,7 +47,7 @@
     UnableToLegalize,
   };
 
-  LegalizerHelper(MachineFunction &MF);
+  LegalizerHelper(MachineFunction &MF, const TargetPassConfig &TPC);
 
   /// Replace \p MI by a sequence of legal instructions that can implement the
   /// same operation. Note that this means \p MI may be deleted, so any iterator
@@ -96,6 +97,7 @@
   MachineIRBuilder MIRBuilder;
   MachineRegisterInfo &MRI;
   const LegalizerInfo &LI;
+  const TargetPassConfig &TPC; // Needed for reportGISelFailure
 };
 
 } // End namespace llvm.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31503.93506.patch
Type: text/x-patch
Size: 3845 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170330/094c8662/attachment.bin>