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

Compiler creates unnecessary pointer to the value on ownership transfering. #125554

Closed
Jerrody opened this issue May 25, 2024 · 7 comments
Closed
Labels
A-codegen Area: Code generation C-discussion Category: Discussion or questions that doesn't represent real issues.

Comments

@Jerrody
Copy link
Contributor

Jerrody commented May 25, 2024

Behaviour is the same in debug and release.

Code

fn main() {
    let vec1 = vec![1, 2, 3];
    println!("{:p}", &vec1);
    let vec2 = vec1;
    println!("{:p}", &vec2);
}

Output

0x7ffe2bd06e90
0x7ffe2bd06e70

This is just waste of CPU time on performing this machinery, where basically changes only identifier.

@Jerrody Jerrody added the C-bug Category: This is a bug. label May 25, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 25, 2024
@workingjubilee
Copy link
Contributor

workingjubilee commented May 25, 2024

Behaviour is the same in debug and release.

Code

fn main() {
    let vec1 = vec![1, 2, 3];
    println!("{:p}", &vec1);
    let vec2 = vec1;
    println!("{:p}", &vec2);
}

Output

0x7ffe2bd06e90 0x7ffe2bd06e70

This is just waste of CPU time on performing this machinery, where basically changes only identifier.

Do you have a version of this that does not require printing the pointer values?

Consider this Godbolt: https://rust.godbolt.org/z/79vGhfcMv

I refer to your code as "print-move-print.rs".

I offer these alternatives, one where we "move" the vec and print the new address twice:

// print-after-move.rs
#[inline(never)]
pub fn main() {
    let vec1 = vec![1, 2, 3];
    let vec2 = vec1;
    println!("{:p}", &vec2);
    println!("{:p}", &vec2);
}

And one where we keep the vec in the same variable and print its address twice:

// stay-printing.rs
#[inline(never)]
pub fn main() {
    let vec2 = vec![1, 2, 3];
    println!("{:p}", &vec2);
    println!("{:p}", &vec2);
}

The last is essentially the "optimized form" that you would want. Note the diff views, that show that the two alternatives I offer are considered equivalent.

It may be hard to follow all the assembly, but note that your version has this string:

        sub     rsp, 128

My versions have this string:

        sub     rsp, 96

That's the size of the stack allocation for locals... and is 32 bytes! ...or the size of a Vec and a reference. I believe your attempt to inspect the address is actually what is producing the deoptimization. I have advocated quite strongly for optimizing the resulting assembly output of Rust code based on move semantics and declaring UB any programs that try to futz with obtaining pointers by strange means, yet it is still not clear to me that this is a desirable optimization after someone prints the address?

@workingjubilee workingjubilee added the A-codegen Area: Code generation label May 25, 2024
@Jerrody
Copy link
Contributor Author

Jerrody commented May 25, 2024

Behaviour is the same in debug and release.

Code

fn main() {
    let vec1 = vec![1, 2, 3];
    println!("{:p}", &vec1);
    let vec2 = vec1;
    println!("{:p}", &vec2);
}

Output

0x7ffe2bd06e90 0x7ffe2bd06e70
This is just waste of CPU time on performing this machinery, where basically changes only identifier.

Do you have a version of this that does not require printing the pointer values?

Consider this Godbolt: https://rust.godbolt.org/z/79vGhfcMv

I refer to your code as "print-move-print.rs".

I offer these alternatives, one where we "move" the vec and print the new address twice:

// print-after-move.rs
#[inline(never)]
pub fn main() {
    let vec1 = vec![1, 2, 3];
    let vec2 = vec1;
    println!("{:p}", &vec2);
    println!("{:p}", &vec2);
}

And one where we keep the vec in the same variable and print its address twice:

// stay-printing.rs
#[inline(never)]
pub fn main() {
    let vec2 = vec![1, 2, 3];
    println!("{:p}", &vec2);
    println!("{:p}", &vec2);
}

The last is essentially the "optimized form" that you would want. Note the diff views, that show that the two alternatives I offer are considered equivalent.

It may be hard to follow all the assembly, but note that your version has this string:

        sub     rsp, 128

My versions have this string:

        sub     rsp, 96

That's the size of the stack allocation for locals... and is 32 bytes! ...or the size of a Vec and a reference. I believe your attempt to inspect the address is actually what is producing the deoptimization. I have advocated quite strongly for optimizing the resulting assembly output of Rust code based on move semantics and declaring UB any programs that try to futz with obtaining pointers by strange means, yet it is still not clear to me that this is a desirable optimization after someone prints the address?

What I would expected in this situation is keeping same pointer to the Vec, instead Rust creates copy of that pointer that points to the Vec. I understand that this example is simple and trivial, but in big applications can happen many moves and this is, little, but unnecessary copy that CPU does, but in reality it can just do nothing and ownership isn't violating, just changes identifier.

@workingjubilee
Copy link
Contributor

@Jerrody Please examine my examples again.

The optimization you describe already occurs, but not if you print out the address.

@Jerrody
Copy link
Contributor Author

Jerrody commented May 25, 2024

@Jerrody Please examine my examples again.

The optimization you describe already occurs, but not if you print out the address.

Alright, another question, why it happens that if I print an address before move and after move then I get different address. I already understood that we get this optimization, but in my example, why it's different.

@the8472
Copy link
Member

the8472 commented May 25, 2024

What jublee is saying is that by trying to observe how the program behaves (printing the addresses) you're altering its behavior. If you don't print the addresses then multiple uses of vec do optimize.

Here's a simpler version with lots of moves: https://rust.godbolt.org/z/Y9xo64o57.
This does optimize, even at opt-level 0!

If we add something that takes a reference but does not leak its address bits then the moves still optimizes away: https://rust.godbolt.org/z/bM8vPc5Ea
The extra mov instrunctions in the assembly are just the length getting black-boxed, not the vec being moved.

I already understood that we get this optimization, but in my example, why it's different.

Printing the address means the address bits now become observable program behavior.
Behavior that is not observable¹ can be modified or eliminated by the optimizer under the as-if rule. Which means that not printing the address gives the optimizer more freedom.

¹according to the language specification, things like debuggers do not count

@workingjubilee
Copy link
Contributor

The problem with your desired optimization, @Jerrody, is ultimately because of this program:

// pointer-escape-kinda.rs
use std::{error, ptr};
#[derive(Debug)]
struct MoveArray<T, const N: usize>([T; N]);

#[inline(never)]
pub fn main() -> Result<(), Box<dyn error::Error>> {
    let place_0 = MoveArray::<i32, 3>([1, 2, 3]);
    let ptr = ptr::addr_of!(place_0) as *mut i32;
    let place_1 = place_0;
    unsafe { ptr.write(5) };
    println!("{:?}", unsafe { (ptr as *mut [i32; 3]).read() });
    println!("{:?}", &place_1);
    Ok(())
}

Note that I do something that one might normally consider "impossible": mutate an immutable location. Indeed, if you run it via Tools -> Miri, Miri will complain about it being UB, currently. This being true makes some other semantics harder to implement, so whether this should be permitted is considered an open question in Rust's operational semantics:

It's not implausible, though, due to the fact you derive the pointer from &Vec<i32>, which has less questions than the ptr::addr_of! I just used.

@Jerrody
Copy link
Contributor Author

Jerrody commented May 26, 2024

The problem with your desired optimization, @Jerrody, is ultimately because of this program:

// pointer-escape-kinda.rs
use std::{error, ptr};
#[derive(Debug)]
struct MoveArray<T, const N: usize>([T; N]);

#[inline(never)]
pub fn main() -> Result<(), Box<dyn error::Error>> {
    let place_0 = MoveArray::<i32, 3>([1, 2, 3]);
    let ptr = ptr::addr_of!(place_0) as *mut i32;
    let place_1 = place_0;
    unsafe { ptr.write(5) };
    println!("{:?}", unsafe { (ptr as *mut [i32; 3]).read() });
    println!("{:?}", &place_1);
    Ok(())
}

Note that I do something that one might normally consider "impossible": mutate an immutable location. Indeed, if you run it via Tools -> Miri, Miri will complain about it being UB, currently. This being true makes some other semantics harder to implement, so whether this should be permitted is considered an open question in Rust's operational semantics:

It's not implausible, though, due to the fact you derive the pointer from &Vec<i32>, which has less questions than the ptr::addr_of! I just used.

Thanks for the explanation. Sorry that I'm dumb in understanding these things, thanks for patient.

@Jerrody Jerrody closed this as completed May 26, 2024
@saethlin saethlin added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation C-discussion Category: Discussion or questions that doesn't represent real issues.
Projects
None yet
Development

No branches or pull requests

5 participants