[GlobalISel] Print/Parse FailedISel MachineFunction property

[rtereshin]

Summary

FailedISel MachineFunction property is part of the CodeGen pipeline state as much as every other property, notably, Legalized, RegBankSelected, and Selected. Let's make that part of the state also serializable / de-serializable, so if GlobalISel aborts on some of the functions of a large module, but not the others, it could be easily seen and the state of the pipeline could be maintained through llc's invocations with -stop-after / -start-after.

To make MIR printable and generally to not to break it too much too soon, this patch also defers cleaning up the vreg -> LLT map until ResetMachineFunctionPass.

To make MIR with FailedISel: true also machine verifiable, machine verifier is changed so it treats a MIR-module as non-regbankselected and non-selected if there is FailedISel property set.

[qcolombet]

Hi Roman.

Looks good to me, but it is missing a test case.
Basically, just something that stops at some point where we can see the FailedISel property and something that parses it (e.g., by checking we run SDISel)

Cheers,
-Quentin

[rtereshin]

In D42877#997945, @qcolombet wrote:

Hi Roman.

Looks good to me, but it is missing a test case.
Basically, just something that stops at some point where we can see the FailedISel property and something that parses it (e.g., by checking we run SDISel)

Cheers,
-Quentin

Hi Quentin,

makes a lot of sense, I'll add something, please hang tight!

Best,
Roman

[rtereshin]

I've added a test that IMO tells a complete user story around this patch, and made necessary changes to make it work.

[rtereshin]

Ping

[dsanders]

LGTM with a couple commit message nits:

• Please mention the machine verifier change that prevents it verifying when it's going to be reset.
• Please mention the deferral of the type map reset

[rtereshin]

@dsanders

Thanks! I've updated the summary here

include/llvm/CodeGen/MIRYamlMapping.h

 struct MachineFunction {
   StringRef Name;
   unsigned Alignment = 0;
   bool ExposesReturnsTwice = false;
   // GISel MachineFunctionProperties.
   bool Legalized = false;
   bool RegBankSelected = false;
   bool Selected = false;
+  bool FailedISel = false;
   // Register information
   bool TracksRegLiveness = false;
   std::vector<VirtualRegisterDefinition> VirtualRegisters;
   std::vector<MachineFunctionLiveIn> LiveIns;
   Optional<std::vector<FlowStringValue>> CalleeSavedRegisters;
   // TODO: Serialize the various register masks.
   // Frame information
   MachineFrameInfo FrameInfo;
   std::vector<FixedMachineStackObject> FixedStackObjects;
   std::vector<MachineStackObject> StackObjects;
   std::vector<MachineConstantPoolValue> Constants; /// Constant pool.
   MachineJumpTable JumpTableInfo;
   BlockStringValue Body;
 };
 
 template <> struct MappingTraits<MachineFunction> {
   static void mapping(IO &YamlIO, MachineFunction &MF) {
     YamlIO.mapRequired("name", MF.Name);
     YamlIO.mapOptional("alignment", MF.Alignment, (unsigned)0);
     YamlIO.mapOptional("exposesReturnsTwice", MF.ExposesReturnsTwice, false);
     YamlIO.mapOptional("legalized", MF.Legalized, false);
     YamlIO.mapOptional("regBankSelected", MF.RegBankSelected, false);
     YamlIO.mapOptional("selected", MF.Selected, false);
+    YamlIO.mapOptional("failedISel", MF.FailedISel, false);
     YamlIO.mapOptional("tracksRegLiveness", MF.TracksRegLiveness, false);
     YamlIO.mapOptional("registers", MF.VirtualRegisters,
                        std::vector<VirtualRegisterDefinition>());
     YamlIO.mapOptional("liveins", MF.LiveIns,
                        std::vector<MachineFunctionLiveIn>());
     YamlIO.mapOptional("calleeSavedRegisters", MF.CalleeSavedRegisters,
                        Optional<std::vector<FlowStringValue>>());
     YamlIO.mapOptional("frameInfo", MF.FrameInfo, MachineFrameInfo());

lib/CodeGen/GlobalISel/InstructionSelect.cpp

 //===- llvm/CodeGen/GlobalISel/InstructionSelect.cpp - InstructionSelect ---==//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 /// \file
 /// This file implements the InstructionSelect class.
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/GlobalISel/InstructionSelect.h"
 #include "llvm/ADT/PostOrderIterator.h"
-#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/CodeGen/GlobalISel/InstructionSelector.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/TargetLowering.h"
 #include "llvm/CodeGen/TargetPassConfig.h"

 }
 
 void InstructionSelect::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<TargetPassConfig>();
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
 bool InstructionSelect::runOnMachineFunction(MachineFunction &MF) {
-  const MachineRegisterInfo &MRI = MF.getRegInfo();
-
-  // No matter what happens, whether we successfully select the function or not,
-  // nothing is going to use the vreg types after us.  Make sure they disappear.
-  auto ClearVRegTypesOnReturn =
-      make_scope_exit([&]() { MRI.getVRegToType().clear(); });
-
   // If the ISel pipeline failed, do not bother running that pass.
   if (MF.getProperties().hasProperty(
           MachineFunctionProperties::Property::FailedISel))
     return false;
 
   DEBUG(dbgs() << "Selecting function: " << MF.getName() << '\n');
 
   const TargetPassConfig &TPC = getAnalysis<TargetPassConfig>();
   const InstructionSelector *ISel = MF.getSubtarget().getInstructionSelector();
   CodeGenCoverage CoverageInfo;
   assert(ISel && "Cannot work without InstructionSelector");
 
   // An optimization remark emitter. Used to report failures.
   MachineOptimizationRemarkEmitter MORE(MF, /*MBFI=*/nullptr);
 
   // FIXME: There are many other MF/MFI fields we need to initialize.
 
+  const MachineRegisterInfo &MRI = MF.getRegInfo();
 #ifndef NDEBUG
   // Check that our input is fully legal: we require the function to have the
   // Legalized property, so it should be.
   // FIXME: This should be in the MachineVerifier, but it can't use the
   // LegalizerInfo as it's currently in the separate GlobalISel library.
   // The RegBankSelected property is already checked in the verifier. Note
   // that it has the same layering problem, but we only use inline methods so
   // end up not needing to link against the GlobalISel library.
#endif
 
   CoverageInfo.emit(CoveragePrefix,
                     MF.getSubtarget()
                         .getTargetLowering()
                         ->getTargetMachine()
                         .getTarget()
                         .getBackendName());
 
+  // If we successfully selected the function nothing is going to use the vreg
+  // types after us (otherwise MIRPrinter would need them). Make sure the types
+  // disappear.
+  MRI.getVRegToType().clear();
+
   // FIXME: Should we accurately track changes?
   return true;
 }

lib/CodeGen/MIRParser/MIRParser.cpp

MIRParserImpl::initializeMachineFunction(const yaml::MachineFunction &YamlMF,
 
   if (YamlMF.Legalized)
     MF.getProperties().set(MachineFunctionProperties::Property::Legalized);
   if (YamlMF.RegBankSelected)
     MF.getProperties().set(
         MachineFunctionProperties::Property::RegBankSelected);
   if (YamlMF.Selected)
     MF.getProperties().set(MachineFunctionProperties::Property::Selected);
+  if (YamlMF.FailedISel)
+    MF.getProperties().set(MachineFunctionProperties::Property::FailedISel);
 
   PerFunctionMIParsingState PFS(MF, SM, IRSlots, Names2RegClasses,
                                 Names2RegBanks);
   if (parseRegisterInfo(PFS, YamlMF))
     return true;
   if (!YamlMF.Constants.empty()) {
     auto *ConstantPool = MF.getConstantPool();
     assert(ConstantPool && "Constant pool must be created");

lib/CodeGen/MIRPrinter.cpp

void MIRPrinter::print(const MachineFunction &MF) {
   YamlMF.ExposesReturnsTwice = MF.exposesReturnsTwice();
 
   YamlMF.Legalized = MF.getProperties().hasProperty(
       MachineFunctionProperties::Property::Legalized);
   YamlMF.RegBankSelected = MF.getProperties().hasProperty(
       MachineFunctionProperties::Property::RegBankSelected);
   YamlMF.Selected = MF.getProperties().hasProperty(
       MachineFunctionProperties::Property::Selected);
+  YamlMF.FailedISel = MF.getProperties().hasProperty(
+      MachineFunctionProperties::Property::FailedISel);
 
   convert(YamlMF, MF.getRegInfo(), MF.getSubtarget().getRegisterInfo());
   ModuleSlotTracker MST(MF.getFunction().getParent());
   MST.incorporateFunction(MF.getFunction());
   convert(MST, YamlMF.FrameInfo, MF.getFrameInfo());
   convertStackObjects(YamlMF, MF, MST);
   if (const auto *ConstantPool = MF.getConstantPool())
     convert(YamlMF, *ConstantPool);

lib/CodeGen/MachineVerifier.cpp

unsigned MachineVerifier::verify(MachineFunction &MF) {
   foundErrors = 0;
 
   this->MF = &MF;
   TM = &MF.getTarget();
   TII = MF.getSubtarget().getInstrInfo();
   TRI = MF.getSubtarget().getRegisterInfo();
   MRI = &MF.getRegInfo();
 
-  isFunctionRegBankSelected = MF.getProperties().hasProperty(
+  const bool isFunctionFailedISel = MF.getProperties().hasProperty(
+      MachineFunctionProperties::Property::FailedISel);
+  isFunctionRegBankSelected =
+      !isFunctionFailedISel &&
+      MF.getProperties().hasProperty(
           MachineFunctionProperties::Property::RegBankSelected);
-  isFunctionSelected = MF.getProperties().hasProperty(
+  isFunctionSelected = !isFunctionFailedISel &&
+                       MF.getProperties().hasProperty(
                            MachineFunctionProperties::Property::Selected);
-
   LiveVars = nullptr;
   LiveInts = nullptr;
   LiveStks = nullptr;
   Indexes = nullptr;
   if (PASS) {
     LiveInts = PASS->getAnalysisIfAvailable<LiveIntervals>();
     // We don't want to verify LiveVariables if LiveIntervals is available.
     if (!LiveInts)

lib/CodeGen/ResetMachineFunctionPass.cpp

 //===-- ResetMachineFunctionPass.cpp - Reset Machine Function ----*- C++ -*-==//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 /// \file
 /// This file implements a pass that will conditionally reset a machine
 /// function as if it was just created. This is used to provide a fallback
 /// mechanism when GlobalISel fails, thus the condition for the reset to
 /// happen is that the MachineFunction has the FailedISel property.
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/Support/Debug.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "reset-machine-function"
 
 STATISTIC(NumFunctionsReset, "Number of functions reset");
public:
     ResetMachineFunction(bool EmitFallbackDiag = false,
                          bool AbortOnFailedISel = false)
         : MachineFunctionPass(ID), EmitFallbackDiag(EmitFallbackDiag),
           AbortOnFailedISel(AbortOnFailedISel) {}
 
     StringRef getPassName() const override { return "ResetMachineFunction"; }
 
     bool runOnMachineFunction(MachineFunction &MF) override {
+      // No matter what happened, whether we successfully selected the function
+      // or not, nothing is going to use the vreg types after us. Make sure they
+      // disappear.
+      auto ClearVRegTypesOnReturn =
+          make_scope_exit([&MF]() { MF.getRegInfo().getVRegToType().clear(); });
+
       if (MF.getProperties().hasProperty(
               MachineFunctionProperties::Property::FailedISel)) {
         if (AbortOnFailedISel)
           report_fatal_error("Instruction selection failed");
         DEBUG(dbgs() << "Resetting: " << MF.getName() << '\n');
         ++NumFunctionsReset;
         MF.reset();
         if (EmitFallbackDiag) {

test/CodeGen/MIR/AArch64/print-parse-verify-failedISel-property.mir

+# RUN: llc -mtriple aarch64-- -run-pass instruction-select -simplify-mir \
+# RUN:     -verify-machineinstrs %s -o - | FileCheck %s
+#
+# RUN: llc -mtriple aarch64-- -global-isel=true -global-isel-abort=2 \
+# RUN:     -start-after=regbankselect -stop-before=expand-isel-pseudos \
+# RUN:     -simplify-mir -verify-machineinstrs %s -o - 2>&1 \
+# RUN:    | FileCheck %s --check-prefix=FALLBACK
+
+# Test that:
+# 1) MIRParser can deserialize FailedISel property.
+# 2) Machine Verifier respects FailedISel and doesn't complain needlessly.
+# 3) MIRPrinter is able to print FailedISel MIR after InstructionSelect pass.
+# 4) MIRPrinter can serialize FailedISel property.
+# 5) It's possible to start llc mid-GlobalISel pipeline from a MIR file with
+#    the FailedISel property set to true and watch it properly fallback to
+#    FastISel / SelectionDAG ISel.
+--- |
+  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64--"
+
+  define i32 @test(i32 %a, i32 %b) #0 {
+  entry:
+    %add = add i32 %b, %a
+    ret i32 %add
+  }
+
+  attributes #0 = { nounwind readnone ssp }
+...
+---
+# CHECK-LABEL: name: test
+# CHECK: failedISel: true
+#
+# FALLBACK: warning: Instruction selection used fallback path for test
+# FALLBACK-LABEL: name: test
+# FALLBACK-NOT: failedISel
+name:            test
+alignment:       2
+legalized:       true
+regBankSelected: true
+failedISel:      true
+tracksRegLiveness: true
+body:             |
+  bb.1.entry:
+    liveins: $w0, $w1
+
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+    ; CHECK: [[COPY1:%[0-9]+]]:_(s32) = COPY $w1
+    ; CHECK: [[ADD:%[0-9]+]]:gpr(s32) = G_ADD [[COPY1]], [[COPY]]
+    ; CHECK: $w0 = COPY [[ADD]](s32)
+    ; CHECK: RET_ReallyLR implicit $w0
+    ;
+    ; FALLBACK: liveins: $w0, $w1
+    ; FALLBACK: [[COPY:%[0-9]+]]:gpr32 = COPY $w1
+    ; FALLBACK: [[COPY1:%[0-9]+]]:gpr32 = COPY $w0
+    ; FALLBACK: [[ADDWrr:%[0-9]+]]:gpr32 = ADDWrr [[COPY]], [[COPY1]]
+    ; FALLBACK: $w0 = COPY [[ADDWrr]]
+    ; FALLBACK: RET_ReallyLR implicit $w0
+
+    %0:_(s32) = COPY $w0
+    %1:_(s32) = COPY $w1
+    %2:gpr(s32) = G_ADD %1, %0
+    $w0 = COPY %2(s32)
+    RET_ReallyLR implicit $w0
+...