Page MenuHomePhabricator

[Time-report] Add a flag -ftime-report={per-pass,per-pass-run} to control the pass timing aggregation
ClosedPublic

Authored by ychen on Dec 1 2020, 4:23 PM.

Details

Summary

Currently, -ftime-report + new pass manager emits one line of report for each
pass run. This potentially causes huge output text especially with regular LTO
or large single file (Obeserved in private tests and was reported in D51276).
The behaviour of -ftime-report + legacy pass manager is
emitting one line of report for each pass object which has relatively reasonable
text output size. This patch adds a flag -ftime-report= to control time report
aggregation for new pass manager.

The flag is for new pass manager only. Using it with legacy pass manager gives
an error. It is a driver and cc1 flag. per-pass is the new default so
-ftime-report is aliased to -ftime-report=per-pass. Before this patch,
functionality-wise -ftime-report is aliased to -ftime-report=per-pass-run.

  • Adds an boolean variable TimePassesHandler::PerRun to control per-pass vs per-pass-run.
  • Adds a new clang CodeGen flag CodeGenOptions::TimePassesPerRun to work with the existing CodeGenOptions::TimePasses.
  • Remove FrontendOptions::ShowTimers, its uses are replaced by the existing CodeGenOptions::TimePasses.
  • Remove FrontendTimesIsEnabled (It was introduced in D45619 which was largely reverted.)

Diff Detail

Event Timeline

ychen created this revision.Dec 1 2020, 4:23 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptDec 1 2020, 4:23 PM
ychen requested review of this revision.Dec 1 2020, 4:23 PM
ychen updated this revision to Diff 309020.Dec 2 2020, 11:25 AM
  • rebase
ychen updated this revision to Diff 309618.Dec 4 2020, 12:34 PM
  • fix rebase error
asbirlea accepted this revision.Dec 7 2020, 2:02 PM

lgtm.

This revision is now accepted and ready to land.Dec 7 2020, 2:02 PM
aeubanks accepted this revision.Dec 7 2020, 2:46 PM

lgtm with comments

clang/test/Misc/time-passes.c
3

this probably needs -fno-experimental-new-pass-manager so that when the NPM is the default this doesn't fail

llvm/lib/IR/PassTimingInfo.cpp
47

is this necessary?

llvm/test/Other/time-passes.ll
55

check that this doesn't appear multiple times under TIME-NEW-PER-PASS?

ychen updated this revision to Diff 310079.Dec 7 2020, 8:13 PM
ychen marked an inline comment as done.
  • clang/test/Misc/time-passes.c: add -fno-experimental-new-pass-manager
  • llvm/test/Other/time-passes.ll: makes sure each pass has one entry.
ychen added inline comments.Dec 7 2020, 8:15 PM
llvm/lib/IR/PassTimingInfo.cpp
47

Yeah, it is. TimePassesPerRun (corresponding to TimePassesHandler::PerRun) is not used if TimePassesIsEnabled is false.

llvm/test/Other/time-passes.ll
55

This is important. Thanks.

aeubanks added inline comments.Dec 7 2020, 8:27 PM
llvm/lib/IR/PassTimingInfo.cpp
47

Whoops, didn't see that this was touching two separate variables

This revision was landed with ongoing or failed builds.Dec 8 2020, 10:14 AM
This revision was automatically updated to reflect the committed changes.