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

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:73-79
+    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();
----------------
This is probably a moot point now, now that it shows that using the "std::pair<LegalizeResult, MachineInstr*>"-approach looks a lot more promising, but I think this code probably still suffers the same problem as before:
After legalizeInstrStep, TmpMI can be removed from the MachineBasicBlock, and TmpMI->getParent() will then probably return a nullptr, causing a segmentation fault.

I'm afraid I don't have a good insight on whether we'd still want to be able to differentiate between "AlreadyLegal" and "Legalized" in the "std::pair<LegalizeResult, MachineInstr*>"-approach.


Repository:
  rL LLVM

https://reviews.llvm.org/D31503