Skip to content

Conversation

AlexanderPortland
Copy link
Contributor

@AlexanderPortland AlexanderPortland commented Jun 24, 2025

Context

When transforming MIR, we use both TransformPass-es that operate over a single body and GlobalPass-es that require further context thus and operate over the whole program. Since these global passes could potentially modify the bodies they operate over, we currently re-run reachability after them to ensure that any changes are reflected in our reachability output.

However, our global passes are largely gated behind unstable features (e.g. -Z uninit-checks to check for uninitialized memory) or specific user actions (e.g. compiling w/ RUSTFLAGS="--emit mir"), so this extra reachability collection is often done unnecessarily. Furthermore, some global passes (like emitting the generated MIR) do not modify the bodies they interact with at all, so re-doing collection is unneeded even when they are enabled.

Changes

This PR modifies the GlobalPass trait to return a boolean representing if the pass modified the MIR in a way that could affect reachability. We then use the boolean flags for each result to redo reachability analysis only if a GlobalPass could have affected its result.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

@AlexanderPortland AlexanderPortland requested a review from a team as a code owner June 24, 2025 20:12
@github-actions github-actions bot added Z-EndToEndBenchCI Tag a PR to run benchmark CI Z-CompilerBenchCI Tag a PR to run benchmark CI labels Jun 24, 2025
@AlexanderPortland
Copy link
Contributor Author

For context, I intentionally implemented this as separate traits rather than having GlobalPass::transform() return a boolean as we do for TransformPass::transform() (this seemed more explicit and elegant type-wise), but I'm very open to feedback and can switch if the latter seems preferable!

tautschnig
tautschnig previously approved these changes Jun 27, 2025
@AlexanderPortland
Copy link
Contributor Author

I actually just switched this to use the same boolean strategy as TransformPass. I love my RefCell<T>s, but this made the changes a lot cleaner without the same runtime overhead.

@carolynzech carolynzech dismissed tautschnig’s stale review June 27, 2025 22:35

Different implementation now, so old review doesn't appy; I can do the re-review next week.

Copy link
Contributor

@carolynzech carolynzech left a comment

Choose a reason for hiding this comment

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

Thanks!

@carolynzech carolynzech added this pull request to the merge queue Jun 30, 2025
Merged via the queue into model-checking:main with commit 5b696e0 Jun 30, 2025
28 checks passed
@carolynzech carolynzech deleted the global-passes branch June 30, 2025 21:25
github-merge-queue bot pushed a commit that referenced this pull request Jul 3, 2025
Auto generated release notes:

## What's Changed
* Edit quantifiers' documentation. by @thanhnguyen-aws in
#4142
* Fix the bug of using multiple hidden variables for the prev of the
same Expr by @thanhnguyen-aws in
#4150
* Remove `assess` subcommand by @carolynzech in
#4111
* Optimize goto binary exporting in `cprover_bindings` by
@AlexanderPortland in #4148
* Add the option to generate performance flamegraphs by
@AlexanderPortland in #4138
* Fix the bug: Static union values can panic Kani by @thanhnguyen-aws in
#4112
* Update toolchain to 2025-06-13 by @carolynzech in
#4152
* Automatic cargo update to 2025-06-16 by @github-actions in
#4156
* Major-version update cargo dependencies by @tautschnig in
#4158
* Upgrade Rust toolchain to 2025-06-16 by @tautschnig in
#4157
* Bump tests/perf/s2n-quic from `3129ad5` to `c6e694e` by @dependabot in
#4160
* Bump tests/perf/s2n-quic from `c6e694e` to `b1b5bf8` by @dependabot in
#4164
* Upgrade Rust toolchain to 2025-06-17 by @tautschnig in
#4163
* Automatic cargo update to 2025-06-23 by @github-actions in
#4172
* Stub panics during MIR transformation by @AlexanderPortland in
#4169
* Bump tests/perf/s2n-quic from `b1b5bf8` to `32ba87d` by @dependabot in
#4175
* Handle enums with zero or one variants by @zhassan-aws in
#4171
* Introduce compiler timing script & CI job by @AlexanderPortland in
#4154
* Upgrade Rust toolchain to 2025-06-18 by @tautschnig in
#4166
* Cache dependencies for CI jobs by @AlexanderPortland in
#4181
* Autoharness: Derive `Arbitrary` for structs and enums by @carolynzech
in #4167
* Upgrade Rust toolchain to 2025-06-27 by @tautschnig in
#4182
* Include wget in dependencies by @zhassan-aws in
#4183
* Automatic cargo update to 2025-06-30 by @github-actions in
#4186
* Add support for loop assigns in loop contracts by @thanhnguyen-aws in
#4174
* Upgrade toolchain to 06/30 by @carolynzech in
#4188
* Optimize reachability with non-mutating global passes by
@AlexanderPortland in #4177
* Bump tests/perf/s2n-quic from `32ba87d` to `b8f8cca` by @dependabot in
#4190
* Bump ncipollo/release-action from 1.16.0 to 1.18.0 by @dependabot in
#4191
* Upgrade toolchain to 07/02 by @carolynzech in
#4195
* Automatic Derivation Fixes by @carolynzech in
#4194


**Full Changelog**:
kani-0.63.0...kani-0.64.0

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-CompilerBenchCI Tag a PR to run benchmark CI Z-EndToEndBenchCI Tag a PR to run benchmark CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants