This is an archive of the discontinued LLVM Phabricator instance.

Skip analysis re-computation when no changes are reported
ClosedPublic

Authored by serge-sans-paille on Aug 24 2020, 3:28 AM.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2020, 3:28 AM
serge-sans-paille requested review of this revision.Aug 24 2020, 3:28 AM

@nikic I'm curious about any binary/compilation time change?

foad added inline comments.Aug 24 2020, 3:32 AM
llvm/lib/Analysis/CallGraphSCCPass.cpp
492–493

This (and line 473) should probably test a "LocalChanged" flag which is ORed into Changed?

llvm/lib/Analysis/RegionPass.cpp
138–139

Use a LocalChanged flag here and line 101?

Take review into account.

serge-sans-paille marked 2 inline comments as done.Aug 25 2020, 12:32 AM
foad added a comment.Aug 25 2020, 1:10 AM

Looks good, but do we also need an equivalent of D80916 to check that passes are reporting "changed" correctly?

Looks good, but do we also need an equivalent of D80916 to check that passes are reporting "changed" correctly?

I've got this patch locally, it raises three errors. I'll submit the patch, the passes update once fixed and when everything lands we can finally consider landing this patch :-)

Thanks for the positive feedback!

nikic accepted this revision.Aug 28 2020, 9:11 AM

LGTM

This revision is now accepted and ready to land.Aug 28 2020, 9:11 AM
nikic added a comment.Aug 28 2020, 2:03 PM

Hrm, the committed patches showed text size changes at O3, so probably something isn't right here: https://llvm-compile-time-tracker.com/compare.php?from=5f1cad4d296a20025f0b3ea30d82b278ddb8779b&to=2296182181521c0b1803a0cd10b098fcfdab1c92&stat=size-text

Relevant files are CMakeFiles/SPASS.dir/renaming.c.o, CMakeFiles/7zip-benchmark.dir/CPP/7zip/Archive/Common/CoderMixer2MT.cpp.o

nikic added a comment.Aug 28 2020, 2:10 PM

Okay, this is not related to this patch after all. I see a couple of text size flips in consecutive commits, so some (other) fairly recent change must have introduced compiler non-determinism :(