This is an archive of the discontinued LLVM Phabricator instance.

Add -stats-stats option
ClosedPublic

Authored by MatzeB on Sep 21 2016, 7:26 PM.

Details

Summary

This option behaves in a similar spirit as -save-temps and writes out
llvm statistics in json format.

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 72135.Sep 21 2016, 7:26 PM
MatzeB retitled this revision from to Add -stats-file option.
MatzeB updated this object.
MatzeB added reviewers: bruno, rsmith, aprantl.
MatzeB set the repository for this revision to rL LLVM.
MatzeB added a subscriber: cfe-commits.
aprantl added inline comments.Sep 22 2016, 8:49 AM
include/clang/Basic/DiagnosticFrontendKinds.td
111 ↗(On Diff #72135)

statstic"s"? not sure.

lib/Driver/Tools.cpp
6093 ↗(On Diff #72135)

ditto :-)

test/Driver/save-stats.c
12 ↗(On Diff #72135)

This is up to taste, but just accepting {{.}} as the path separator would be sufficient IMO and might be more readable.

bruno added inline comments.Sep 23 2016, 11:28 AM
lib/Driver/Tools.cpp
6102 ↗(On Diff #72135)

Why removing StatsFile here? IIUC, at this point StatsFile is still the same as the output (if it's a file).

MatzeB added inline comments.Sep 23 2016, 11:33 AM
lib/Driver/Tools.cpp
6102 ↗(On Diff #72135)

a) It behaves like -save-temps=obj (clang -save-temps=obj foo.c -o bar will give you foo.i, foo.o, ...;
b) This makes sense when you have multiple (clang -save-stats=obj foo.c bar.c -o baz) inputs for which you need multiple .stats filenames but you only have 1 output name
c) It magically works for -o - as well

bruno edited edge metadata.Sep 23 2016, 2:19 PM

Maybe add some docs to explain the new flags?

lib/Driver/Tools.cpp
6102 ↗(On Diff #72135)

Ok, I see now, I misread remove_filename as remove

test/Driver/save-stats.c
1 ↗(On Diff #72135)

Is -save-stats == -save-stats=cwd? It doesn't seem so by looking at lib/Driver/Tools.cpp. Need test for the diag::err_drv_invalid_value as well.

12 ↗(On Diff #72135)

+1 to Adrian's suggestion

MatzeB updated this revision to Diff 72376.Sep 23 2016, 3:58 PM
MatzeB edited edge metadata.
MatzeB marked an inline comment as done.

Thanks for the reviews.

Addressed comments.

MatzeB retitled this revision from Add -stats-file option to Add -stats-stats option.Sep 23 2016, 3:58 PM
MatzeB added inline comments.
test/Driver/save-stats.c
1 ↗(On Diff #72135)

Yes, that aspect is handled by the options parser, look at the changes in Options.td

bruno accepted this revision.Sep 26 2016, 9:47 AM
bruno edited edge metadata.

Thanks Matthias

LGTM!

This revision is now accepted and ready to land.Sep 26 2016, 9:47 AM
This revision was automatically updated to reflect the committed changes.
MatzeB added inline comments.Sep 26 2016, 1:54 PM
test/Driver/save-stats.c
12 ↗(On Diff #72135)

For the record: Switching to {{.}} in the final commit broke the windows bots, because they display an escaped slash here (= two backslash characters)...

aprantl edited edge metadata.Sep 26 2016, 5:17 PM

Awesome. Sorry!