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

[Aditya Nandakumar via Phabricator via llvm-commits]

aditya_nandakumar added a comment.

In https://reviews.llvm.org/D31503#714507, @ab wrote:

> I considered and dismissed speculatively printing:  it's a pretty expensive operation (especially when it needs to print an IR value, and it needs to recompute slot numbers).
>
> Here's another idea I considered:  have legalizeInstr return the instruction it failed on, in a std::pair<LegalizeResult, MachineInstr*>.
>
> In practice, the external users of the LegalizeHelper API shouldn't care about LegalizeResult: we either return UnableToLegalize, or we return a success value (Legalized/AlreadyLegal.  So we could simply return a MachineInstr*, which is nullptr if we succeeded.  It would be weird to read though, so we could go extra fancy and have an Error that either contains a MachineInstr*, or is Error::success().


Just to clarify here, (IIUC) returning either an Error (with MachineInstr*) or Error::success() would mean that we'd not be able to differentiate b/w Legalized and AlreadyLegal. Did you mean Expected<LegalizeResult>?


Repository:
  rL LLVM

https://reviews.llvm.org/D31503