Extend -Zdump-mir output with two-phase borrow info #105815

pnkfelix · 2022-12-17T03:54:28Z

This should cause no breakage since its just injecting a comment /*2pb*/ (and the -Zdump-mir output is unstable anyway).

…::Unique while we're at it.

rustbot · 2022-12-17T03:54:34Z

r? @lcnr

(rustbot has picked a reviewer for you, use r? to override)

lcnr · 2022-12-19T07:24:54Z

generally in favor even though the representation isn't great. I am not sure how long it would take me to figure out what /* 2pb */ means. Can't quickly think of a better alternative though '^^

r? @oli-obk

oli-obk · 2022-12-19T12:39:54Z

We could include more information in the // comments after the statement, by adding an arm for Rvalue::Ref in

rust/compiler/rustc_middle/src/mir/pretty.rs

Line 501 in 3af7df9

fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) {

pnkfelix · 2022-12-28T15:24:48Z

@oli-obk can you spell out how much more info you think you'd want? Right now this really is just a boolean flag, so the presence/absence of a tag like /*2pb*/ is all I could think of to include.

(Maybe you're saying you'd want to know about when the reservation begins versus when the exclusive access itself actually begins, which I would agree would be superior, but I do not think that information is directly reflected in the MIR structure, and I think I'd bias towards making the dump_mir output merely reflect existing structure rather than have it attempt further deductions in the dump_mir code itself...)

pnkfelix · 2022-12-28T15:27:24Z

(I have been musing whether there would be value in having the MIR itself use general structure where every mutable borrow is modelled as a two-phase borrow, with distinct "reserve-mut" and "exercise-the-reservation" operations. But that is a huge change that would need far broader discussion.)

oli-obk · 2023-01-09T12:41:07Z

Right now this really is just a boolean flag, so the presence/absence of a tag like /*2pb*/ is all I could think of to include.

Oh, I don't mean more information, just clarifying what 2pb means in a comment, similar to how we have comments for constants giving their type and potentially verbose value

oli-obk · 2023-01-10T08:29:29Z

r=me with either the new comments added or not ^^

@rustbot author

JohnCSimon · 2023-04-30T04:00:30Z

@pnkfelix Ping from triage: Can you post your status on this PR? This has sat idle for a few months.

JohnCSimon · 2023-12-17T21:35:17Z

@pnkfelix

Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

…s, r=wesleywiser Stabilize -Zno-jump-tables into -Cjump-tables=bool I propose stabilizing the -Zno-jump-tables option into -Cjump-tables=<bool>. # `-Zno-jump-tables` stabilization report ## What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized? No RFC was created for this option. This was a narrowly scoped option introduced in rust-lang#105812 to support code generation requirements of the x86-64 linux kernel, and eventually other targets as Rust For Linux grows. The tracking is rust-lang#116592. ## What behavior are we committing to that has been controversial? Summarize the major arguments pro/con. The behavior of this flag is well defined, and mimics the existing `-fno-jump-tables` option currently available with LLVM and GCC with some caveats: * Unlike clang or gcc, this option may be ignored by the code generation backend. Rust can support multiple code-generation backends. For stabilization, only the LLVM backend honors this option. * The usage of this option will not guarantee a library or binary is free of jump tables. To ensure a jump-table free binary, all crates in the build graph must be compiled with this option. This includes implicitly linked crates such as std or core. * This option only enforces the crate being compiled is free of jump tables. * No verification is done to ensure other crates are compiled with this option. Enforcing code generation options are applied across the crate graph is out of scope for this option. What should the flag name be? * As introduced, this option was named `-Zno-jump-tables`. However, other major toolchains allow both positive and negative variants of this option to toggle this feature. Renaming the option to `-Cjump-tables=<bool>` makes this option consistent, and if for some reason, expandable to other arguments in the future. Notably, many LLVM targets have a configurable and different thresholds for when to lower into a jump table. ## Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those. No. This option is used exclusively to gate a very specific class of optimization. ## Summarize the major parts of the implementation and provide links into the code (or to PRs) * The original PR rust-lang#105812 by `@ojeda` * The stabilized CLI option is parsed as a bool: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_session/src/options.rs#L2025-L2026 * This options adds an attribute to each llvm function via: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_codegen_llvm/src/attributes.rs#L210-L215 * Finally, the rustc book is updated with the new option: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/src/doc/rustc/src/codegen-options/index.md?plain=1#L212-L223 ## Has a call-for-testing period been conducted? If so, what feedback was received? No. The option has originally created is being used by Rust For Linux to build the x86-64 kernel without issue. ## What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking? There are no outstanding issues. ## Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization * `@ojeda` implemented this feature in rust-lang#105815 as `-Zno-jump-tables`. * `@tgross35` created and maintained the tracking issue rust-lang#116592, and provided feedback about the naming of the cli option. ## What FIXMEs are still in the code for that feature and why is it ok to leave them there? There are none. ## What static checks are done that are needed to prevent undefined behavior? This option cannot cause undefined behavior. It is a boolean option with well defined behavior in both cases. ## In what way does this feature interact with the reference/specification, and are those edits prepared? This adds a new cli option to `rustc`. The documentation is updated, and the unstable documentation cleaned up in this PR. ## Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries? No. ## What other unstable features may be exposed by this feature? None. ## What is tooling support like for this feature, w.r.t rustdoc, clippy, rust-analzyer, rustfmt, etc.? No support is required from other rust tooling. ## Open Items - [x] Are there objections renaming `-Zno-jump-tables` to `-Cjump-tables=<bool>`? The consensus is no. - [x] Is it desirable to keep `-Zno-jump-tables` for a period of time? The consensus is no. --- Closes rust-lang#116592

…s, r=wesleywiser Stabilize -Zno-jump-tables into -Cjump-tables=bool I propose stabilizing the -Zno-jump-tables option into -Cjump-tables=<bool>. # `-Zno-jump-tables` stabilization report ## What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized? No RFC was created for this option. This was a narrowly scoped option introduced in rust-lang#105812 to support code generation requirements of the x86-64 linux kernel, and eventually other targets as Rust For Linux grows. The tracking is rust-lang#116592. ## What behavior are we committing to that has been controversial? Summarize the major arguments pro/con. The behavior of this flag is well defined, and mimics the existing `-fno-jump-tables` option currently available with LLVM and GCC with some caveats: * Unlike clang or gcc, this option may be ignored by the code generation backend. Rust can support multiple code-generation backends. For stabilization, only the LLVM backend honors this option. * The usage of this option will not guarantee a library or binary is free of jump tables. To ensure a jump-table free binary, all crates in the build graph must be compiled with this option. This includes implicitly linked crates such as std or core. * This option only enforces the crate being compiled is free of jump tables. * No verification is done to ensure other crates are compiled with this option. Enforcing code generation options are applied across the crate graph is out of scope for this option. What should the flag name be? * As introduced, this option was named `-Zno-jump-tables`. However, other major toolchains allow both positive and negative variants of this option to toggle this feature. Renaming the option to `-Cjump-tables=<bool>` makes this option consistent, and if for some reason, expandable to other arguments in the future. Notably, many LLVM targets have a configurable and different thresholds for when to lower into a jump table. ## Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those. No. This option is used exclusively to gate a very specific class of optimization. ## Summarize the major parts of the implementation and provide links into the code (or to PRs) * The original PR rust-lang#105812 by ``@ojeda`` * The stabilized CLI option is parsed as a bool: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_session/src/options.rs#L2025-L2026 * This options adds an attribute to each llvm function via: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_codegen_llvm/src/attributes.rs#L210-L215 * Finally, the rustc book is updated with the new option: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/src/doc/rustc/src/codegen-options/index.md?plain=1#L212-L223 ## Has a call-for-testing period been conducted? If so, what feedback was received? No. The option has originally created is being used by Rust For Linux to build the x86-64 kernel without issue. ## What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking? There are no outstanding issues. ## Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization * ``@ojeda`` implemented this feature in rust-lang#105815 as `-Zno-jump-tables`. * ``@tgross35`` created and maintained the tracking issue rust-lang#116592, and provided feedback about the naming of the cli option. ## What FIXMEs are still in the code for that feature and why is it ok to leave them there? There are none. ## What static checks are done that are needed to prevent undefined behavior? This option cannot cause undefined behavior. It is a boolean option with well defined behavior in both cases. ## In what way does this feature interact with the reference/specification, and are those edits prepared? This adds a new cli option to `rustc`. The documentation is updated, and the unstable documentation cleaned up in this PR. ## Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries? No. ## What other unstable features may be exposed by this feature? None. ## What is tooling support like for this feature, w.r.t rustdoc, clippy, rust-analzyer, rustfmt, etc.? No support is required from other rust tooling. ## Open Items - [x] Are there objections renaming `-Zno-jump-tables` to `-Cjump-tables=<bool>`? The consensus is no. - [x] Is it desirable to keep `-Zno-jump-tables` for a period of time? The consensus is no. --- Closes rust-lang#116592

…s, r=wesleywiser Stabilize -Zno-jump-tables into -Cjump-tables=bool I propose stabilizing the -Zno-jump-tables option into -Cjump-tables=<bool>. # `-Zno-jump-tables` stabilization report ## What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized? No RFC was created for this option. This was a narrowly scoped option introduced in rust-lang#105812 to support code generation requirements of the x86-64 linux kernel, and eventually other targets as Rust For Linux grows. The tracking is rust-lang#116592. ## What behavior are we committing to that has been controversial? Summarize the major arguments pro/con. The behavior of this flag is well defined, and mimics the existing `-fno-jump-tables` option currently available with LLVM and GCC with some caveats: * Unlike clang or gcc, this option may be ignored by the code generation backend. Rust can support multiple code-generation backends. For stabilization, only the LLVM backend honors this option. * The usage of this option will not guarantee a library or binary is free of jump tables. To ensure a jump-table free binary, all crates in the build graph must be compiled with this option. This includes implicitly linked crates such as std or core. * This option only enforces the crate being compiled is free of jump tables. * No verification is done to ensure other crates are compiled with this option. Enforcing code generation options are applied across the crate graph is out of scope for this option. What should the flag name be? * As introduced, this option was named `-Zno-jump-tables`. However, other major toolchains allow both positive and negative variants of this option to toggle this feature. Renaming the option to `-Cjump-tables=<bool>` makes this option consistent, and if for some reason, expandable to other arguments in the future. Notably, many LLVM targets have a configurable and different thresholds for when to lower into a jump table. ## Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those. No. This option is used exclusively to gate a very specific class of optimization. ## Summarize the major parts of the implementation and provide links into the code (or to PRs) * The original PR rust-lang#105812 by ```@ojeda``` * The stabilized CLI option is parsed as a bool: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_session/src/options.rs#L2025-L2026 * This options adds an attribute to each llvm function via: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_codegen_llvm/src/attributes.rs#L210-L215 * Finally, the rustc book is updated with the new option: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/src/doc/rustc/src/codegen-options/index.md?plain=1#L212-L223 ## Has a call-for-testing period been conducted? If so, what feedback was received? No. The option has originally created is being used by Rust For Linux to build the x86-64 kernel without issue. ## What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking? There are no outstanding issues. ## Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization * ```@ojeda``` implemented this feature in rust-lang#105815 as `-Zno-jump-tables`. * ```@tgross35``` created and maintained the tracking issue rust-lang#116592, and provided feedback about the naming of the cli option. ## What FIXMEs are still in the code for that feature and why is it ok to leave them there? There are none. ## What static checks are done that are needed to prevent undefined behavior? This option cannot cause undefined behavior. It is a boolean option with well defined behavior in both cases. ## In what way does this feature interact with the reference/specification, and are those edits prepared? This adds a new cli option to `rustc`. The documentation is updated, and the unstable documentation cleaned up in this PR. ## Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries? No. ## What other unstable features may be exposed by this feature? None. ## What is tooling support like for this feature, w.r.t rustdoc, clippy, rust-analzyer, rustfmt, etc.? No support is required from other rust tooling. ## Open Items - [x] Are there objections renaming `-Zno-jump-tables` to `-Cjump-tables=<bool>`? The consensus is no. - [x] Is it desirable to keep `-Zno-jump-tables` for a period of time? The consensus is no. --- Closes rust-lang#116592

Rollup merge of #145974 - pmur:murp/stabilize-zno-jump-tables, r=wesleywiser Stabilize -Zno-jump-tables into -Cjump-tables=bool I propose stabilizing the -Zno-jump-tables option into -Cjump-tables=<bool>. # `-Zno-jump-tables` stabilization report ## What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized? No RFC was created for this option. This was a narrowly scoped option introduced in #105812 to support code generation requirements of the x86-64 linux kernel, and eventually other targets as Rust For Linux grows. The tracking is #116592. ## What behavior are we committing to that has been controversial? Summarize the major arguments pro/con. The behavior of this flag is well defined, and mimics the existing `-fno-jump-tables` option currently available with LLVM and GCC with some caveats: * Unlike clang or gcc, this option may be ignored by the code generation backend. Rust can support multiple code-generation backends. For stabilization, only the LLVM backend honors this option. * The usage of this option will not guarantee a library or binary is free of jump tables. To ensure a jump-table free binary, all crates in the build graph must be compiled with this option. This includes implicitly linked crates such as std or core. * This option only enforces the crate being compiled is free of jump tables. * No verification is done to ensure other crates are compiled with this option. Enforcing code generation options are applied across the crate graph is out of scope for this option. What should the flag name be? * As introduced, this option was named `-Zno-jump-tables`. However, other major toolchains allow both positive and negative variants of this option to toggle this feature. Renaming the option to `-Cjump-tables=<bool>` makes this option consistent, and if for some reason, expandable to other arguments in the future. Notably, many LLVM targets have a configurable and different thresholds for when to lower into a jump table. ## Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those. No. This option is used exclusively to gate a very specific class of optimization. ## Summarize the major parts of the implementation and provide links into the code (or to PRs) * The original PR #105812 by ```@ojeda``` * The stabilized CLI option is parsed as a bool: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_session/src/options.rs#L2025-L2026 * This options adds an attribute to each llvm function via: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_codegen_llvm/src/attributes.rs#L210-L215 * Finally, the rustc book is updated with the new option: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/src/doc/rustc/src/codegen-options/index.md?plain=1#L212-L223 ## Has a call-for-testing period been conducted? If so, what feedback was received? No. The option has originally created is being used by Rust For Linux to build the x86-64 kernel without issue. ## What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking? There are no outstanding issues. ## Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization * ```@ojeda``` implemented this feature in #105815 as `-Zno-jump-tables`. * ```@tgross35``` created and maintained the tracking issue #116592, and provided feedback about the naming of the cli option. ## What FIXMEs are still in the code for that feature and why is it ok to leave them there? There are none. ## What static checks are done that are needed to prevent undefined behavior? This option cannot cause undefined behavior. It is a boolean option with well defined behavior in both cases. ## In what way does this feature interact with the reference/specification, and are those edits prepared? This adds a new cli option to `rustc`. The documentation is updated, and the unstable documentation cleaned up in this PR. ## Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries? No. ## What other unstable features may be exposed by this feature? None. ## What is tooling support like for this feature, w.r.t rustdoc, clippy, rust-analzyer, rustfmt, etc.? No support is required from other rust tooling. ## Open Items - [x] Are there objections renaming `-Zno-jump-tables` to `-Cjump-tables=<bool>`? The consensus is no. - [x] Is it desirable to keep `-Zno-jump-tables` for a period of time? The consensus is no. --- Closes #116592

pnkfelix added 3 commits December 14, 2022 15:11

include two-phase borrow info in MIR dump output.

a0025a9

address tidy's concerns, and add an aside about output for BorrowKind…

d68329c

…::Unique while we're at it.

Update test MIR output to reflect the addition of the /*2pb*/ marker.

4164551

rustbot assigned lcnr Dec 17, 2022

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 Dec 17, 2022

rustbot assigned oli-obk and unassigned lcnr Dec 19, 2022

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 Jan 10, 2023

JohnCSimon closed this Dec 17, 2023

rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Dec 17, 2023

pmur mentioned this pull request Aug 28, 2025

Stabilize -Zno-jump-tables into -Cjump-tables=bool #145974

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Extend -Zdump-mir output with two-phase borrow info #105815

Extend -Zdump-mir output with two-phase borrow info #105815

Uh oh!

pnkfelix commented Dec 17, 2022

Uh oh!

rustbot commented Dec 17, 2022

Uh oh!

lcnr commented Dec 19, 2022

Uh oh!

oli-obk commented Dec 19, 2022

Uh oh!

pnkfelix commented Dec 28, 2022

Uh oh!

pnkfelix commented Dec 28, 2022 •

edited

Loading

Uh oh!

oli-obk commented Jan 9, 2023

Uh oh!

oli-obk commented Jan 10, 2023

Uh oh!

JohnCSimon commented Apr 30, 2023

Uh oh!

JohnCSimon commented Dec 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Extend -Zdump-mir output with two-phase borrow info #105815

Extend -Zdump-mir output with two-phase borrow info #105815

Uh oh!

Conversation

pnkfelix commented Dec 17, 2022

Uh oh!

rustbot commented Dec 17, 2022

Uh oh!

lcnr commented Dec 19, 2022

Uh oh!

oli-obk commented Dec 19, 2022

Uh oh!

pnkfelix commented Dec 28, 2022

Uh oh!

pnkfelix commented Dec 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oli-obk commented Jan 9, 2023

Uh oh!

oli-obk commented Jan 10, 2023

Uh oh!

JohnCSimon commented Apr 30, 2023

Uh oh!

JohnCSimon commented Dec 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pnkfelix commented Dec 28, 2022 •

edited

Loading