]> granicus.if.org Git - llvm/commitdiff
[GlobalISel]: Fix bug where we can report GISelFailure on erased instructions
authorAditya Nandakumar <aditya_nandakumar@apple.com>
Fri, 7 Apr 2017 21:49:30 +0000 (21:49 +0000)
committerAditya Nandakumar <aditya_nandakumar@apple.com>
Fri, 7 Apr 2017 21:49:30 +0000 (21:49 +0000)
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

include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
lib/CodeGen/GlobalISel/Legalizer.cpp
lib/CodeGen/GlobalISel/LegalizerHelper.cpp
test/CodeGen/AArch64/GlobalISel/gisel-fail-intermediate-legalizer.ll [new file with mode: 0644]

index ec8e141893e7c92b61fb889bdb20db463175c96c..8fecafdc08d0e9fdf1eba436716413feda1f7b50 100644 (file)
@@ -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;
 };
index 849363caf09c372203b9576a2b195ca6f9385eea..657ddb30791952af164493ff81a6a93bebe83993 100644 (file)
@@ -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) {
index 17b7e294b6b01bb8ac700500b745fab45e7a31cf..afab23d94f0dbf3156e76c0720b33c2cbc3a7286 100644 (file)
@@ -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 (file)
index 0000000..e333f74
--- /dev/null
@@ -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
+}