Skip to content

Add tracing to validate_operand #143051

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Stypox
Copy link
Contributor

@Stypox Stypox commented Jun 26, 2025

This PR adds a tracing call to keep track of how much time is spent in validate_operand and const_validate_operand. Let me know if more fine-grained tracing is needed (e.g. adding tracing to validate_operand_internal too, which is just called from those two functions).

I also fixed the rustdoc of validate_operand and const_validate_operand since it was referencing an older name for the val parameter which was renamed in cbdcbf0.

Here is some tracing output when Miri is run on src/tools/miri/tests/pass/hello.rs, visualizable in ui.perfetto.dev: trace-1750932222218210.json

Note: obtaining tracing output depends on rust-lang/miri#4406, but this PR is standalone and can be merged without waiting for rust-lang/miri#4406.

r? @RalfJung

The name of the parameter changed from `op` to `val` in cbdcbf0
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 26, 2025
@Stypox Stypox force-pushed the tracing-validity branch from bb1d1af to 757380e Compare June 26, 2025 10:07
@RalfJung
Copy link
Member

Note: this PR is not ready yet as it requires rust-lang/miri#4406 to disable the changes in the Miri code when "tracing" is disabled.

What happens if this gets merged without that PR? By default, all tracing is still disabled in Miri, right?

@Stypox Stypox force-pushed the tracing-validity branch from 757380e to 89a636f Compare June 27, 2025 08:15
@Stypox
Copy link
Contributor Author

Stypox commented Jun 27, 2025

What happens if this gets merged without that PR? By default, all tracing is still disabled in Miri, right?

Yes, this can be merged without issues even before rust-lang/miri#4406

@RalfJung
Copy link
Member

Yes, this can be merged without issues even before rust-lang/miri#4406

Okay, then please make the PR & description ready for that. :)

@Stypox
Copy link
Contributor Author

Stypox commented Jun 27, 2025

@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 27, 2025
@Stypox Stypox marked this pull request as ready for review June 27, 2025 09:20
@rustbot
Copy link
Collaborator

rustbot commented Jun 27, 2025

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

@RalfJung
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 27, 2025

📌 Commit 89a636f has been approved by RalfJung

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
4 participants