This is an archive of the discontinued LLVM Phabricator instance.

llvm-reduce: Don't assert on functions which don't track liveness
ClosedPublic

Authored by arsenm on Jun 6 2022, 6:08 AM.

Details

Reviewers
qcolombet
MatzeB
Summary

I still do not understand why this is a property that exists. The
queries for block liveins assert that this is true. However, it's
possible to have block live ins set without this set. In order to
accurately clone the function, we should either remove the assertion
from the liveins() query, or have the verifier check that there are no
block live ins for !TracksLiveness.

Diff Detail

Event Timeline

arsenm created this revision.Jun 6 2022, 6:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2022, 6:08 AM
arsenm requested review of this revision.Jun 6 2022, 6:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2022, 6:08 AM
Herald added a subscriber: wdng. · View Herald Transcript

The background for this property is that we only really compute the livein lists at the end of register allocation (in the VirtRegRewriter or FastRegAlloc). Before that point using the liveins is pretty much always wrong because they are not complete/correct.

Unfortunately the live-ins are also used to model the unusual case for exception handling: Unwinding from a call appears like a jump on the control flow graph; unfortunately contrary to all the other jumps in a CFG the unwinding logic executes real code and happens to set a register to a new value that we want to read in the target block. This puts us into the odd position that we need to model this register being set, but doing so at the end of the predecessor block is wrong and will unnecessarily increase pressure for the non-exception cases, doing at the beginning of the target block is wrong, because the register may not be set when there's multiple predecessor in the target. Setting a register "on" a CFG edge isn't really something we can model. So instead we do this "hack" where we insert the register into the live-in list of the target...

I don't remember the details, but I think I added the assert to all the normal accessor functions to protect people from relying on the live-in information when they shouldn't, but there was at least 1 way to read the live-ins anyway if you really needed to check for that exception handling special case...

arsenm updated this revision to Diff 434632.Jun 6 2022, 3:42 PM

Use non-asserting functions in serialization contexts (which should probably be renamed)

MatzeB accepted this revision.Jun 6 2022, 4:06 PM

oh looks like we also mark the function parameters as live-in causing a bunch of test changes. Anyway I guess this change better reflects the reality of what registers are in the live-in lists. LGTM

This revision is now accepted and ready to land.Jun 6 2022, 4:06 PM