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

generated call to core::fmt::Arguments::new_const fails to constant fold #128709

Open
lolbinarycat opened this issue Aug 5, 2024 · 4 comments
Open
Assignees
Labels
A-fmt Area: `std::fmt` C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@lolbinarycat
Copy link
Contributor

lolbinarycat commented Aug 5, 2024

normal code:

pub fn main() {
    println!("hello");
}

code that should be equivalent, but is actually more efficient

#![feature(print_internals)]
#![feature(const_fmt_arguments_new)]

pub fn main() {
    ::std::io::_print(const { format_args!("hello\n") }); 
}

godbolt link for asm comparison

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 5, 2024
@lolbinarycat
Copy link
Contributor Author

possible workaround: during the code generation at compiler/rustc_ast_lowering/src/format.rs:474, wrap the call expr in a ConstBlock expr.

@SkiFire13
Copy link
Contributor

This is not a valid optimization in general, as soon as format_args captures a variable this becomes invalid.

#![feature(print_internals)]
#![feature(const_fmt_arguments_new)]

pub fn main() {
    let n = 42;
    // println!("hello {n}"); // Compiles fine
    ::std::io::_print(const { format_args!("hello {n}\n") }); // Error
}

@lolbinarycat
Copy link
Contributor Author

yes, but there's already a special case in codegen for when there's no arguments (which happens after the implicit named argument desugaring). this special case is what calls new_const, but it doesn't actually get folded.

@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such labels Aug 13, 2024
@saethlin saethlin added A-fmt Area: `std::fmt` and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 13, 2024
@veera-sivarajan
Copy link
Contributor

@rustbot claim

bors added a commit to rust-lang-ci/rust that referenced this issue Oct 13, 2024
…<try>

Evaluate `std::fmt::Arguments::new_const()` during Compile Time

Fixes rust-lang#128709

This PR aims to optimize calls to string formating macros without any arguments by evaluating `std::fmt::Arguments::new_const()` in a const context.

Currently,
`println!("hola")` compiles to `std::io::_print(std::fmt::Arguments::new_const(&["hola\n"]))`.

With this PR,
`println!("hola")` compiles to `std::io::_print(const { std::fmt::Arguments::new_const(&["hola\n"]) })`.

This is accomplished in two steps:

1.  Const stabilize `std::fmt::Arguments::new_const()`.
2.  Wrap calls to `std::fmt::Arguments::new_const()` in an inline const block when lowering the AST to HIR.

This reduces the generated code to a `memcpy` instead of multiple `getelementptr` and `store` instructions even with `-C no-prepopulate-passes -C opt-level=0`. Godbolt for code comparison: https://rust.godbolt.org/z/P7Px7de6c

This is a safe and sound transformation because `std::fmt::Arguments::new_const()` is a trivial constructor function taking a slice containing a `'static` string literal as input.

CC rust-lang#99012
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 14, 2024
…<try>

Evaluate `std::fmt::Arguments::new_const()` during Compile Time

Fixes rust-lang#128709

This PR aims to optimize calls to string formating macros without any arguments by evaluating `std::fmt::Arguments::new_const()` in a const context.

Currently,
`println!("hola")` compiles to `std::io::_print(std::fmt::Arguments::new_const(&["hola\n"]))`.

With this PR,
`println!("hola")` compiles to `std::io::_print(const { std::fmt::Arguments::new_const(&["hola\n"]) })`.

This is accomplished in two steps:

1.  Const stabilize `std::fmt::Arguments::new_const()`.
2.  Wrap calls to `std::fmt::Arguments::new_const()` in an inline const block when lowering the AST to HIR.

This reduces the generated code to a `memcpy` instead of multiple `getelementptr` and `store` instructions even with `-C no-prepopulate-passes -C opt-level=0`. Godbolt for code comparison: https://rust.godbolt.org/z/P7Px7de6c

This is a safe and sound transformation because `std::fmt::Arguments::new_const()` is a trivial constructor function taking a slice containing a `'static` string literal as input.

CC rust-lang#99012
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 28, 2024
…<try>

Evaluate `std::fmt::Arguments::new_const()` during Compile Time

Fixes rust-lang#128709

This PR aims to optimize calls to string formating macros without any arguments by evaluating `std::fmt::Arguments::new_const()` in a const context.

Currently,
`println!("hola")` compiles to `std::io::_print(std::fmt::Arguments::new_const(&["hola\n"]))`.

With this PR,
`println!("hola")` compiles to `std::io::_print(const { std::fmt::Arguments::new_const(&["hola\n"]) })`.

This is accomplished by wrapping calls to `std::fmt::Arguments::new_const()` in an inline const block when lowering the AST to HIR.

This reduces the generated code to a `memcpy` instead of multiple `getelementptr` and `store` instructions even with `-C no-prepopulate-passes -C opt-level=0`. Godbolt for code comparison: https://rust.godbolt.org/z/P7Px7de6c

This is a safe and sound transformation because `std::fmt::Arguments::new_const()` is a trivial constructor function taking a slice containing a `'static` string literal as input.

CC rust-lang#99012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-fmt Area: `std::fmt` C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants