Page MenuHomePhabricator

[ThinLTO] Detect partially split modules during the thin link
ClosedPublic

Authored by tejohnson on Jan 31 2019, 6:46 PM.

Details

Summary

The changes to disable LTO unit splitting by default (r350949) and
detect inconsistently split LTO units (r350948) are causing some crashes
when the inconsistency is detected in multiple threads simultaneously.
Fix that by having the code always look for the inconsistently split
LTO units during the thin link, by checking for the presence of type
tests recorded in the summaries.

Modify test added in r350948 to remove single threading required to fix
a bot failure due to this issue (and some debugging options added in the
process of diagnosing it).

Diff Detail

Repository
rL LLVM

Event Timeline

tejohnson created this revision.Jan 31 2019, 6:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 31 2019, 6:46 PM
pcc added a comment.Jan 31 2019, 7:43 PM

Can we do this in LTO.cpp maybe? If we do that, we can return a nice error message to the user via the Error interface. The message should probably also suggest recompiling with -fsplit-lto-unit.

tejohnson updated this revision to Diff 184801.Feb 1 2019, 11:22 AM

Address suggestion: move error checking to LTO.cpp

pcc accepted this revision.Feb 14 2019, 9:49 AM

LGTM

lib/LTO/LTO.cpp
828 ↗(On Diff #184801)

A user who sees this message isn't going to understand what llvm.type.test or llvm.type.checked.load means. Maybe omit this part of the error?

845 ↗(On Diff #184801)

Same here.

This revision is now accepted and ready to land.Feb 14 2019, 9:49 AM
tejohnson marked 2 inline comments as done.Feb 14 2019, 1:21 PM
tejohnson updated this revision to Diff 186908.Feb 14 2019, 1:22 PM

Address comment

This revision was automatically updated to reflect the committed changes.