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

Improve codegen for single argument format_args! #75301

Open
pickfire opened this issue Aug 8, 2020 · 4 comments
Open

Improve codegen for single argument format_args! #75301

pickfire opened this issue Aug 8, 2020 · 4 comments
Labels
A-fmt Area: `std::fmt` C-bug Category: This is a bug. I-heavy Issue: Problems and improvements with respect to binary size of generated code. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pickfire
Copy link
Contributor

pickfire commented Aug 8, 2020

I was using write! in a lot of places, some I just do write!(writer, " ") but I noticed that the generated assembly is not as good as writer.write_all(b" "). Since we have minimal const fn now, I wonder if we could improve the generated assembly when a single character or a single argument is passed in.

use std::io::{self, Write};

pub fn write_char(buf: &mut Vec<u8>) -> io::Result<()> {
    write!(buf, " ")
}
use std::io::{self, Write};

pub fn write_char(buf: &mut Vec<u8>) -> io::Result<()> {
    buf.write_all(b" ")
}

https://rust.godbolt.org/z/zzEMjd

I also write!() could accept a single byte or character. Example, write!(buf, ' ').

If anyone can mentor, maybe I can take on this part (hopefully without compiling rust).

Meta

rustc --version --verbose:

nigthly

CC @lzutao

@pickfire pickfire added the C-bug Category: This is a bug. label Aug 8, 2020
@tesuji
Copy link
Contributor

tesuji commented Aug 10, 2020

I opened one in #75358

@rustbot rustbot added I-heavy Issue: Problems and improvements with respect to binary size of generated code. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 10, 2020
@RalfJung
Copy link
Member

RalfJung commented Aug 12, 2020

This is entirely unrelated to const fn. const fn doesn't change assembly, it just changes which code you can write

const FOO: i32 = { HERE };

However, I agree a string-only format_args! could likely be improved.

Another case that could possibly be improved is

panic_fmt(fmt::Arguments::new_v1(&[expr], &[]));

@workingjubilee
Copy link
Member

workingjubilee commented Sep 8, 2020

Seems related to #10761

I think the best thing is to try to nudge the write! macro itself to try to handle this, because waiting until it reaches write_fmt seems to damage the codegen, as write_fmt then has more overhead to examine. This should be handle-able at the macro/const eval time, in effect.

@workingjubilee workingjubilee added the A-fmt Area: `std::fmt` label Apr 25, 2021
@CAD97
Copy link
Contributor

CAD97 commented Jul 22, 2022

Did fmt::Arguments::as_str improve the situation here? If it's not being used by write! impls yet, it probably can/should be.

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-bug Category: This is a bug. I-heavy Issue: Problems and improvements with respect to binary size of generated code. I-slow Issue: Problems and improvements with respect to performance of generated code. 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