This is an archive of the discontinued LLVM Phabricator instance.

RegAlloc: try to fail more gracefully when out of registers
ClosedPublic

Authored by nhaehnle on May 3 2019, 5:00 AM.

Details

Summary

The emitError path allows the program to continue, unlike report_fatal_error.
This is friendlier to use cases where LLVM is embedded in a larger program,
because the caller may be able to deal with the error somewhat gracefully.

Change the number of requested NOP bytes in the AArch64 and PowerPC
test cases to avoid triggering an unrelated assertion. The compilation
still fails, as verified by the test.

Change-Id: Iafb9ca341002a597b82e59ddc7a1f13c78758e3d

Diff Detail

Repository
rL LLVM

Event Timeline

nhaehnle created this revision.May 3 2019, 5:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 3 2019, 5:00 AM
arsenm accepted this revision.May 3 2019, 5:41 AM

LGTM. Note I think this is also broken in fast regalloc. I'm also aware of a crash there on out of registers, which I was hoping to fix after the rewrite series is (finally) committed

This revision is now accepted and ready to land.May 3 2019, 5:41 AM
qcolombet added inline comments.May 3 2019, 8:57 AM
lib/CodeGen/RegAllocBase.cpp
129 ↗(On Diff #197957)

Although I find the patch sensible, the new behavior is somewhat misleading:
If MI is not an inline asm statement, we are still going to report a source location (if any), whereas unlike inline asm statement, there is little to nothing that the user can do with that information. It could actually be counter productive to give them that location.

arsenm added inline comments.May 3 2019, 9:22 AM
lib/CodeGen/RegAllocBase.cpp
129 ↗(On Diff #197957)

It should probably report the source location for the function, rather than the instruction in that case

I think there's a fundamental distinction between the two cases of running out of registers. We expect that the user of a programming language will write inline asm directives, and some of those directives will require more registers than the architecture actually supports. But for other instructions, the user isn't writing the register inputs/outputs explicitly; we can't run out of registers unless there's a compiler bug (either in the register allocator, or elsewhere).

Given that, there needs to be some way for error reporting to distinguish the two errors. When we run out of registers due to a non-inline-asm construct in clang, we want to make it clear to the user that the issue is a compiler bug, not user error. This means we need to trigger the code in the clang driver that asks for a bug report and generates preprocessed source.

Of course, that isn't fundamentally tied to calling abort() from the register allocator; some alternate mechanism for reporting ICEs which doesn't immediately kill the process would be reasonable.

Given that, there needs to be some way for error reporting to distinguish the two errors. When we run out of registers due to a non-inline-asm construct in clang, we want to make it clear to the user that the issue is a compiler bug, not user error. This means we need to trigger the code in the clang driver that asks for a bug report and generates preprocessed source.

Of course, that isn't fundamentally tied to calling abort() from the register allocator; some alternate mechanism for reporting ICEs which doesn't immediately kill the process would be reasonable.

That seems reasonable, though presumably something for a separate change.

nhaehnle updated this revision to Diff 198416.May 7 2019, 2:14 AM

Changing the error reporting not to report the instruction location
in the non-inline asm case.

I've changed the patch to not report the location in the non-inline case on the basis that it's an internal compiler error. Will keep it up for a while here and commit if there are no further comments.

This revision was automatically updated to reflect the committed changes.