Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustc_llvm: Add a -Z print-llvm-stats option to expose LLVM statistics. #104000

Closed
wants to merge 2 commits into from

Conversation

pcwalton
Copy link
Contributor

@pcwalton pcwalton commented Nov 5, 2022

LLVM has a neat statistics feature that tracks how often optimizations kick in. It's very handy for optimization work. Since we expose the LLVM pass timings, I thought it made sense to expose the LLVM statistics too.

r? @oli-obk

…ics.

LLVM has a neat [statistics] feature that tracks how often optimizations kick
in. It's very handy for optimization work. Since we expose the LLVM pass
timings, I thought it made sense to expose the LLVM statistics too.

[statistics]: https://llvm.org/docs/ProgrammersManual.html#the-statistic-class-stats-option
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 5, 2022
@rustbot
Copy link
Collaborator

rustbot commented Nov 5, 2022

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

@@ -1465,6 +1465,9 @@ options! {
#[rustc_lint_opt_deny_field_access("use `Session::print_llvm_passes` instead of this field")]
print_llvm_passes: bool = (false, parse_bool, [UNTRACKED],
"print the LLVM optimization passes being run (default: no)"),
#[rustc_lint_opt_deny_field_access("use `Session::print_llvm_stats` instead of this field")]
print_llvm_stats: bool = (false, parse_bool, [UNTRACKED],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe call it print_codegen_stats since it's forwarded to all ssa codegen backends

@@ -113,6 +114,11 @@ extern "C" void LLVMRustPrintPassTimings() {
TimerGroup::printAll(OS);
}

extern "C" void LLVMRustPrintStatistics() {
raw_fd_ostream OS(2, false); // stderr.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn’t it be better to return a string from this function and leave it up to rustc to actually print? Sure it’ll allocate the stats into a memory buffer, but that seems like a negligible cost.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same is true for LLVMRustPrintPassTimings I guess, but that's preexisting.

What would be the benefit? Better reusability?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, one thing I’m thinking of is that just dumping stuff onto stderr is not a great experience with output formats like JSON. It just seems that leaving outputting to rustc is just gonna leave less room for things to go very wrong in general. While I’m not anticipating this getting fixed anytime soon, but having this just output a string seems like a meaningful intermediate step.

But also, looking at that magic number makes my eyeballs really itchy, even despite the comment :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nagisa I'm not opposed to doing it that way (allocating into a string). Do you want me to change LLVMRustPrintPassTimings too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be quite awesome!

@oli-obk
Copy link
Contributor

oli-obk commented Dec 13, 2022

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 13, 2022
@bors
Copy link
Contributor

bors commented Dec 26, 2022

☔ The latest upstream changes (presumably #106143) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon
Copy link
Member

@pcwalton Can you please post your status on this PR? It has sat idle for months.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 17, 2023

closing in favor of the rebased and fixed version at #113723

thanks @khei4 <3

@oli-obk oli-obk closed this Jul 17, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 21, 2023
…nikic

Resurrect: rustc_llvm: Add a -Z `print-codegen-stats` option to expose LLVM statistics.

This resurrects PR rust-lang#104000, which has sat idle for a while. And I want to see the effect of stack-move optimizations on LLVM (like https://reviews.llvm.org/D153453) :).

I have applied the changes requested by `@oli-obk` and `@nagisa`  rust-lang#104000 (comment) and rust-lang#104000 (comment) in the latest commits.

r? `@oli-obk`

-----

LLVM has a neat [statistics](https://llvm.org/docs/ProgrammersManual.html#the-statistic-class-stats-option) feature that tracks how often optimizations kick in. It's very handy for optimization work. Since we expose the LLVM pass timings, I thought it made sense to expose the LLVM statistics too.

-----
(Edit: fix broken link
(Edit2: fix segmentation fault and use malloc

If `rustc` is built with
```toml
[llvm]
assertions = true
```
Then you can see like
```
rustc +stage1 -Z print-codegen-stats -C opt-level=3  tmp.rs
===-------------------------------------------------------------------------===
                          ... Statistics Collected ...
===-------------------------------------------------------------------------===
         3 aa                           - Number of MayAlias results
       193 aa                           - Number of MustAlias results
       531 aa                           - Number of NoAlias results
...
```

And the current default build emits only
```
$ rustc +stage1 -Z print-codegen-stats -C opt-level=3  tmp.rs
===-------------------------------------------------------------------------===
                          ... Statistics Collected ...
===-------------------------------------------------------------------------===
$
```
This might be better to emit the message to tell assertion flag necessity, but now I can't find how to do that...
antoyo pushed a commit to rust-lang/rustc_codegen_gcc that referenced this pull request Aug 13, 2023
Resurrect: rustc_llvm: Add a -Z `print-codegen-stats` option to expose LLVM statistics.

This resurrects PR rust-lang/rust#104000, which has sat idle for a while. And I want to see the effect of stack-move optimizations on LLVM (like https://reviews.llvm.org/D153453) :).

I have applied the changes requested by `@oli-obk` and `@nagisa`  rust-lang/rust#104000 (comment) and rust-lang/rust#104000 (comment) in the latest commits.

r? `@oli-obk`

-----

LLVM has a neat [statistics](https://llvm.org/docs/ProgrammersManual.html#the-statistic-class-stats-option) feature that tracks how often optimizations kick in. It's very handy for optimization work. Since we expose the LLVM pass timings, I thought it made sense to expose the LLVM statistics too.

-----
(Edit: fix broken link
(Edit2: fix segmentation fault and use malloc

If `rustc` is built with
```toml
[llvm]
assertions = true
```
Then you can see like
```
rustc +stage1 -Z print-codegen-stats -C opt-level=3  tmp.rs
===-------------------------------------------------------------------------===
                          ... Statistics Collected ...
===-------------------------------------------------------------------------===
         3 aa                           - Number of MayAlias results
       193 aa                           - Number of MustAlias results
       531 aa                           - Number of NoAlias results
...
```

And the current default build emits only
```
$ rustc +stage1 -Z print-codegen-stats -C opt-level=3  tmp.rs
===-------------------------------------------------------------------------===
                          ... Statistics Collected ...
===-------------------------------------------------------------------------===
$
```
This might be better to emit the message to tell assertion flag necessity, but now I can't find how to do that...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants