[llvm] r299802 - [GlobalISel]: Fix bug where we can report GISelFailure on erased instructions

Aditya Nandakumar via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 7 14:49:30 PDT 2017


Author: aditya_nandakumar
Date: Fri Apr  7 16:49:30 2017
New Revision: 299802

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

Added:
    llvm/trunk/test/CodeGen/AArch64/GlobalISel/gisel-fail-intermediate-legalizer.ll
Modified:
    llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
    llvm/trunk/lib/CodeGen/GlobalISel/Legalizer.cpp
    llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h?rev=299802&r1=299801&r2=299802&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h Fri Apr  7 16:49:30 2017
@@ -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;
 };

Modified: llvm/trunk/lib/CodeGen/GlobalISel/Legalizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/Legalizer.cpp?rev=299802&r1=299801&r2=299802&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/Legalizer.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/Legalizer.cpp Fri Apr  7 16:49:30 2017
@@ -171,21 +171,34 @@ bool Legalizer::runOnMachineFunction(Mac
       // and are assumed to be legal.
       if (!isPreISelGenericOpcode(MI->getOpcode()))
         continue;
+      SmallVector<MachineInstr *, 4> WorkList;
+      Helper.MIRBuilder.recordInsertions(
+          [&](MachineInstr *MI) { WorkList.push_back(MI); });
+      WorkList.push_back(&*MI);
 
-      auto Res = Helper.legalizeInstr(*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());
 
-      // 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;
+      Helper.MIRBuilder.stopRecordingInsertions();
     }
 
-
   MachineRegisterInfo &MRI = MF.getRegInfo();
   const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
   for (auto &MBB : MF) {

Modified: llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp?rev=299802&r1=299801&r2=299802&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Fri Apr  7 16:49:30 2017
@@ -57,31 +57,6 @@ LegalizerHelper::legalizeInstrStep(Machi
   }
 }
 
-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)

Added: llvm/trunk/test/CodeGen/AArch64/GlobalISel/gisel-fail-intermediate-legalizer.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/gisel-fail-intermediate-legalizer.ll?rev=299802&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/gisel-fail-intermediate-legalizer.ll (added)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/gisel-fail-intermediate-legalizer.ll Fri Apr  7 16:49:30 2017
@@ -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
+}




More information about the llvm-commits mailing list