This is an archive of the discontinued LLVM Phabricator instance.

[MustExecute] Use MustBeExecutedInterval to eliminate work duplication
Needs ReviewPublic

Authored by jdoerfert on Apr 16 2020, 1:24 AM.

Details

Summary

Must be executed intervals are stretches of instructions that are
guaranteed to be executed together without any other instruction
executed in-between. In addition, an interval I can end in a link into
another interval J. This means that the instructions of the interval
J, starting from the position the link points to, are always executed
before or respectively after the instruction in I. Note that there
might be other instructions executed in-between linked intervals.

With this patch the must-be-executed-context explorer will created an
overlay of the CFG using partially connected intervals. In contrast to
the old scheme, we do not need to recompute join points or next/previous
instructions more than once, the information is part of the interval
web. Furthermore, iterators are changed to be lightweight, only
containing the position on the forward and backward web of intervals as
well as pointer to the explorer. The latter could potentially be
removed as well.

The interface of the explorer was changed because the users do not need
to create and hold on to iterators anymore.

Early test results show significant improvements wrt. compile time and
no changes to the context.

Diff Detail

Event Timeline

jdoerfert created this revision.Apr 16 2020, 1:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2020, 1:24 AM
jdoerfert updated this revision to Diff 258449.Apr 17 2020, 5:04 PM

Include test changes in the Attributor

jdoerfert updated this revision to Diff 259597.Apr 23 2020, 9:06 AM

rebase and new test changes

@jdoerfert What is happening with this patch?

@jdoerfert What is happening with this patch?

Waiting on review... ping?

Early test results show significant improvements wrt. compile time and no changes to the context.

But the tests are changed.
Can this be split up into NFC refactoring + some other patches that change tests?

lebedev.ri accepted this revision.Jul 10 2020, 3:09 PM

Alright, this has been up for review long enough :)

Large diffs are indeed scary, and impossible to review,
and there is not really a way for people to mark parts
as reviewed so if i'm not the first one reviewing,
i guess no one has spotted anything bad yet.

This is pretty self-contained, in a pretty new (attributor) area.
Cursory examination suggests that this looks about right.
I suspect the perf story is indeed better than what there currently is :S

So i feel like i'm okay with rubber-stamping it.

This revision is now accepted and ready to land.Jul 10 2020, 3:09 PM
lebedev.ri resigned from this revision.Jan 12 2023, 5:43 PM

This review seems to be stuck/dead, consider abandoning if no longer relevant.

This revision now requires review to proceed.Jan 12 2023, 5:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2023, 5:43 PM