This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Print/Parse FailedISel MachineFunction property
ClosedPublic

Authored by rtereshin on Feb 2 2018, 7:12 PM.

Details

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.

Diff Detail

Event Timeline

rtereshin created this revision.Feb 2 2018, 7:12 PM
rtereshin retitled this revision from [GLobalISel] Print/Parse FailedISel MachineFunction property to [GlobalISel] Print/Parse FailedISel MachineFunction property.Feb 2 2018, 7:13 PM

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 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 updated this revision to Diff 134360.Feb 14 2018, 6:52 PM

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

dsanders accepted this revision.Feb 26 2018, 3:32 PM

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
This revision is now accepted and ready to land.Feb 26 2018, 3:32 PM
rtereshin edited the summary of this revision. (Show Details)Feb 26 2018, 3:41 PM

@dsanders

Thanks! I've updated the summary here

This revision was automatically updated to reflect the committed changes.