From da5a0cd69c64e0772960caee32f36b11bca1b498 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 7 Feb 2019 12:33:27 +0100 Subject: [PATCH 1/7] De-duplicate number formatting implementations for smaller code size Instead of inlining the same logic into every number formatting implementation, pull it out into a function that each of the number formatting impls call into. --- src/libcore/fmt/num.rs | 69 +++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/src/libcore/fmt/num.rs b/src/libcore/fmt/num.rs index 3a812337bb111..31886748fa6c1 100644 --- a/src/libcore/fmt/num.rs +++ b/src/libcore/fmt/num.rs @@ -178,7 +178,8 @@ integer! { i32, u32 } integer! { i64, u64 } integer! { i128, u128 } -const DEC_DIGITS_LUT: &'static[u8] = + +static DEC_DIGITS_LUT: &[u8; 200] = b"0001020304050607080910111213141516171819\ 2021222324252627282930313233343536373839\ 4041424344454647484950515253545556575859\ @@ -186,18 +187,8 @@ const DEC_DIGITS_LUT: &'static[u8] = 8081828384858687888990919293949596979899"; macro_rules! impl_Display { - ($($t:ident),*: $conv_fn:ident) => ($( - #[stable(feature = "rust1", since = "1.0.0")] - impl fmt::Display for $t { - #[allow(unused_comparisons)] - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let is_nonnegative = *self >= 0; - let mut n = if is_nonnegative { - self.$conv_fn() - } else { - // convert the negative num to positive by summing 1 to it's 2 complement - (!self.$conv_fn()).wrapping_add(1) - }; + ($($t:ident),* as $u:ident via $conv_fn:ident named $name:ident) => { + fn $name(mut n: $u, is_nonnegative: bool, f: &mut fmt::Formatter) -> fmt::Result { let mut buf = uninitialized_array![u8; 39]; let mut curr = buf.len() as isize; let buf_ptr = MaybeUninit::first_ptr_mut(&mut buf); @@ -205,18 +196,18 @@ macro_rules! impl_Display { unsafe { // need at least 16 bits for the 4-characters-at-a-time to work. - if ::mem::size_of::<$t>() >= 2 { - // eagerly decode 4 characters at a time - while n >= 10000 { - let rem = (n % 10000) as isize; - n /= 10000; + assert!(::mem::size_of::<$u>() >= 2); + + // eagerly decode 4 characters at a time + while n >= 10000 { + let rem = (n % 10000) as isize; + n /= 10000; - let d1 = (rem / 100) << 1; - let d2 = (rem % 100) << 1; - curr -= 4; - ptr::copy_nonoverlapping(lut_ptr.offset(d1), buf_ptr.offset(curr), 2); - ptr::copy_nonoverlapping(lut_ptr.offset(d2), buf_ptr.offset(curr + 2), 2); - } + let d1 = (rem / 100) << 1; + let d2 = (rem % 100) << 1; + curr -= 4; + ptr::copy_nonoverlapping(lut_ptr.offset(d1), buf_ptr.offset(curr), 2); + ptr::copy_nonoverlapping(lut_ptr.offset(d2), buf_ptr.offset(curr + 2), 2); } // if we reach here numbers are <= 9999, so at most 4 chars long @@ -247,15 +238,31 @@ macro_rules! impl_Display { }; f.pad_integral(is_nonnegative, "", buf_slice) } - })*); + + $( + #[stable(feature = "rust1", since = "1.0.0")] + impl fmt::Display for $t { + #[allow(unused_comparisons)] + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let is_nonnegative = *self >= 0; + let n = if is_nonnegative { + self.$conv_fn() + } else { + // convert the negative num to positive by summing 1 to it's 2 complement + (!self.$conv_fn()).wrapping_add(1) + }; + $name(n, is_nonnegative, f) + } + })* + }; } -impl_Display!(i8, u8, i16, u16, i32, u32: to_u32); -impl_Display!(i64, u64: to_u64); -impl_Display!(i128, u128: to_u128); +impl_Display!(i8, u8, i16, u16, i32, u32 as u32 via to_u32 named fmt_u32); +impl_Display!(i64, u64 as u64 via to_u64 named fmt_u64); +impl_Display!(i128, u128 as u128 via to_u128 named fmt_u128); #[cfg(target_pointer_width = "16")] -impl_Display!(isize, usize: to_u16); +impl_Display!(isize, usize as u16 via to_u16 named fmt_usize); #[cfg(target_pointer_width = "32")] -impl_Display!(isize, usize: to_u32); +impl_Display!(isize, usize as u32 via to_u32 named fmt_usize); #[cfg(target_pointer_width = "64")] -impl_Display!(isize, usize: to_u64); +impl_Display!(isize, usize as u64 via to_u64 named fmt_usize); From ed2157a38f6ccdfe460f2f058f60a67daac6cc5a Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 7 Feb 2019 13:02:27 +0100 Subject: [PATCH 2/7] De-duplicate write_prefix lambda in pad_integral For smaller code size. --- src/libcore/fmt/mod.rs | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index 530b2f52c0df2..dd95e3b4a7ce4 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -1153,38 +1153,46 @@ impl<'a> Formatter<'a> { sign = Some('+'); width += 1; } - let prefixed = self.alternate(); - if prefixed { + let prefix = if self.alternate() { width += prefix.chars().count(); - } + Some(prefix) + } else { + None + }; // Writes the sign if it exists, and then the prefix if it was requested - let write_prefix = |f: &mut Formatter| { + #[inline(never)] + fn write_prefix(f: &mut Formatter, sign: Option, prefix: Option<&str>) -> Result { if let Some(c) = sign { f.buf.write_char(c)?; } - if prefixed { f.buf.write_str(prefix) } - else { Ok(()) } - }; + if let Some(prefix) = prefix { + f.buf.write_str(prefix) + } else { + Ok(()) + } + } // The `width` field is more of a `min-width` parameter at this point. match self.width { // If there's no minimum length requirements then we can just // write the bytes. None => { - write_prefix(self)?; self.buf.write_str(buf) + write_prefix(self, sign, prefix)?; + self.buf.write_str(buf) } // Check if we're over the minimum width, if so then we can also // just write the bytes. Some(min) if width >= min => { - write_prefix(self)?; self.buf.write_str(buf) + write_prefix(self, sign, prefix)?; + self.buf.write_str(buf) } // The sign and prefix goes before the padding if the fill character // is zero Some(min) if self.sign_aware_zero_pad() => { self.fill = '0'; self.align = rt::v1::Alignment::Right; - write_prefix(self)?; + write_prefix(self, sign, prefix)?; self.with_padding(min - width, rt::v1::Alignment::Right, |f| { f.buf.write_str(buf) }) @@ -1192,7 +1200,8 @@ impl<'a> Formatter<'a> { // Otherwise, the sign and prefix goes after the padding Some(min) => { self.with_padding(min - width, rt::v1::Alignment::Right, |f| { - write_prefix(f)?; f.buf.write_str(buf) + write_prefix(f, sign, prefix)?; + f.buf.write_str(buf) }) } } From e633f152397545c2fd80795fc928ec555656b2ab Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 7 Feb 2019 15:01:30 +0100 Subject: [PATCH 3/7] Un-monomorphize and inline formatting with padding The generic `F` in `with_padding` was causing a bunch of stuff to get inlined that otherwise needn't be, blowing up code size. --- src/libcore/fmt/mod.rs | 86 ++++++++++++++++++++++++++++-------------- 1 file changed, 57 insertions(+), 29 deletions(-) diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index dd95e3b4a7ce4..605779046ef51 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -1036,6 +1036,32 @@ pub fn write(output: &mut dyn Write, args: Arguments) -> Result { Ok(()) } +/// Padding after the end of something. Returned by `Formatter::padding`. +#[must_use = "don't forget to write the post padding"] +struct PostPadding { + fill: [u8; 4], + fill_len: u32, + padding: usize, +} + +impl PostPadding { + /// Safety relies on `fill[..fill_len]` being a valid UTF-8 char. + unsafe fn new(fill: [u8; 4], fill_len: u32, padding: usize) -> PostPadding { + PostPadding { fill, fill_len, padding } + } + + /// Write this post padding. + fn write(self, buf: &mut dyn Write) -> Result { + let fill = unsafe { + str::from_utf8_unchecked(&self.fill.get_unchecked(..self.fill_len as usize)) + }; + for _ in 0..self.padding { + buf.write_str(fill)?; + } + Ok(()) + } +} + impl<'a> Formatter<'a> { fn wrap_buf<'b, 'c, F>(&'b mut self, wrap: F) -> Formatter<'c> where 'b: 'c, F: FnOnce(&'b mut (dyn Write+'b)) -> &'c mut (dyn Write+'c) @@ -1193,16 +1219,16 @@ impl<'a> Formatter<'a> { self.fill = '0'; self.align = rt::v1::Alignment::Right; write_prefix(self, sign, prefix)?; - self.with_padding(min - width, rt::v1::Alignment::Right, |f| { - f.buf.write_str(buf) - }) + let post_padding = self.padding(min - width, rt::v1::Alignment::Right)?; + self.buf.write_str(buf)?; + post_padding.write(self.buf) } // Otherwise, the sign and prefix goes after the padding Some(min) => { - self.with_padding(min - width, rt::v1::Alignment::Right, |f| { - write_prefix(f, sign, prefix)?; - f.buf.write_str(buf) - }) + let post_padding = self.padding(min - width, rt::v1::Alignment::Right)?; + write_prefix(self, sign, prefix)?; + self.buf.write_str(buf)?; + post_padding.write(self.buf) } } } @@ -1273,19 +1299,21 @@ impl<'a> Formatter<'a> { // up the minimum width with the specified string + some alignment. Some(width) => { let align = rt::v1::Alignment::Left; - self.with_padding(width - s.chars().count(), align, |me| { - me.buf.write_str(s) - }) + let post_padding = self.padding(width - s.chars().count(), align)?; + self.buf.write_str(s)?; + post_padding.write(self.buf) } } } - /// Runs a callback, emitting the correct padding either before or - /// afterwards depending on whether right or left alignment is requested. - fn with_padding(&mut self, padding: usize, default: rt::v1::Alignment, - f: F) -> Result - where F: FnOnce(&mut Formatter) -> Result, - { + /// Write the pre-padding and return the unwritten post-padding. Callers are + /// responsible for ensuring post-padding is written after the thing that is + /// being padded. + fn padding( + &mut self, + padding: usize, + default: rt::v1::Alignment + ) -> result::Result { let align = match self.align { rt::v1::Alignment::Unknown => default, _ => self.align @@ -1299,19 +1327,19 @@ impl<'a> Formatter<'a> { }; let mut fill = [0; 4]; - let fill = self.fill.encode_utf8(&mut fill); - - for _ in 0..pre_pad { - self.buf.write_str(fill)?; - } + let fill_len = { + let fill = self.fill.encode_utf8(&mut fill); - f(self)?; + for _ in 0..pre_pad { + self.buf.write_str(fill)?; + } - for _ in 0..post_pad { - self.buf.write_str(fill)?; - } + fill.len() + }; - Ok(()) + Ok(unsafe { + PostPadding::new(fill, fill_len as u32, post_pad) + }) } /// Takes the formatted parts and applies the padding. @@ -1343,9 +1371,9 @@ impl<'a> Formatter<'a> { let ret = if width <= len { // no padding self.write_formatted_parts(&formatted) } else { - self.with_padding(width - len, align, |f| { - f.write_formatted_parts(&formatted) - }) + let post_padding = self.padding(width - len, align)?; + self.write_formatted_parts(&formatted)?; + post_padding.write(self.buf) }; self.fill = old_fill; self.align = old_align; From c104b5c89767101a40ef173eb84d3aa1122e5e9a Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 7 Feb 2019 15:44:52 +0100 Subject: [PATCH 4/7] Also de-duplicate 32- and 64-bit number formatting on wasm32 --- src/libcore/fmt/num.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/libcore/fmt/num.rs b/src/libcore/fmt/num.rs index 31886748fa6c1..b9fa364037108 100644 --- a/src/libcore/fmt/num.rs +++ b/src/libcore/fmt/num.rs @@ -257,12 +257,22 @@ macro_rules! impl_Display { }; } -impl_Display!(i8, u8, i16, u16, i32, u32 as u32 via to_u32 named fmt_u32); -impl_Display!(i64, u64 as u64 via to_u64 named fmt_u64); +// Include wasm32 in here since it doesn't reflect the native pointer size, and +// often cares strongly about getting a smaller code size. +#[cfg(any(target_pointer_width = "64", target_arch = "wasm32"))] +mod imp { + use super::*; + impl_Display!( + i8, u8, i16, u16, i32, u32, i64, u64, usize, isize + as u64 via to_u64 named fmt_u64 + ); +} + +#[cfg(not(any(target_pointer_width = "64", target_arch = "wasm32")))] +mod imp { + use super::*; + impl_Display!(i8, u8, i16, u16, i32, u32, isize, usize as u32 via to_u32 named fmt_u32); + impl_Display!(i64, u64 as u64 via to_u64 named fmt_u64); +} + impl_Display!(i128, u128 as u128 via to_u128 named fmt_u128); -#[cfg(target_pointer_width = "16")] -impl_Display!(isize, usize as u16 via to_u16 named fmt_usize); -#[cfg(target_pointer_width = "32")] -impl_Display!(isize, usize as u32 via to_u32 named fmt_usize); -#[cfg(target_pointer_width = "64")] -impl_Display!(isize, usize as u64 via to_u64 named fmt_usize); From 05df9ff41537bab6586eea7ad9272eeb611e09e1 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 7 Feb 2019 17:05:23 +0100 Subject: [PATCH 5/7] Add a wasm code size test for stringifying numbers --- .../wasm-stringify-ints-small/Makefile | 10 +++++ .../run-make/wasm-stringify-ints-small/foo.rs | 39 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 src/test/run-make/wasm-stringify-ints-small/Makefile create mode 100755 src/test/run-make/wasm-stringify-ints-small/foo.rs diff --git a/src/test/run-make/wasm-stringify-ints-small/Makefile b/src/test/run-make/wasm-stringify-ints-small/Makefile new file mode 100644 index 0000000000000..18e47242f806b --- /dev/null +++ b/src/test/run-make/wasm-stringify-ints-small/Makefile @@ -0,0 +1,10 @@ +-include ../../run-make-fulldeps/tools.mk + +ifeq ($(TARGET),wasm32-unknown-unknown) +all: + $(RUSTC) foo.rs -C lto -O --target wasm32-unknown-unknown + wc -c < $(TMPDIR)/foo.wasm + [ "`wc -c < $(TMPDIR)/foo.wasm`" -lt "21000" ] +else +all: +endif diff --git a/src/test/run-make/wasm-stringify-ints-small/foo.rs b/src/test/run-make/wasm-stringify-ints-small/foo.rs new file mode 100755 index 0000000000000..6aa249aabeb93 --- /dev/null +++ b/src/test/run-make/wasm-stringify-ints-small/foo.rs @@ -0,0 +1,39 @@ +#![crate_type = "cdylib"] + +extern "C" { + fn observe(ptr: *const u8, len: usize); + + fn get_u8() -> u8; + fn get_i8() -> i8; + fn get_u16() -> u16; + fn get_i16() -> i16; + fn get_u32() -> u32; + fn get_i32() -> i32; + fn get_u64() -> u64; + fn get_i64() -> i64; + fn get_usize() -> usize; + fn get_isize() -> isize; +} + +macro_rules! stringify { + ( $($f:ident)* ) => { + $( + let s = $f().to_string(); + observe(s.as_ptr(), s.len()); + )* + }; +} + +#[no_mangle] +pub unsafe extern "C" fn foo() { + stringify!(get_u8); + stringify!(get_i8); + stringify!(get_u16); + stringify!(get_i16); + stringify!(get_u32); + stringify!(get_i32); + stringify!(get_u64); + stringify!(get_i64); + stringify!(get_usize); + stringify!(get_isize); +} From 8fea7054b9c77a7dcf242ce84cfc0a58d81a9c00 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 8 Feb 2019 10:02:24 +0100 Subject: [PATCH 6/7] Use write_char for writing padding characters Removes some unsafe *and* saves almost half a kilobyte of code size. --- src/libcore/fmt/mod.rs | 30 +++++-------------- .../wasm-stringify-ints-small/Makefile | 2 +- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/src/libcore/fmt/mod.rs b/src/libcore/fmt/mod.rs index 605779046ef51..4c0eb1eeb5530 100644 --- a/src/libcore/fmt/mod.rs +++ b/src/libcore/fmt/mod.rs @@ -1039,24 +1039,19 @@ pub fn write(output: &mut dyn Write, args: Arguments) -> Result { /// Padding after the end of something. Returned by `Formatter::padding`. #[must_use = "don't forget to write the post padding"] struct PostPadding { - fill: [u8; 4], - fill_len: u32, + fill: char, padding: usize, } impl PostPadding { - /// Safety relies on `fill[..fill_len]` being a valid UTF-8 char. - unsafe fn new(fill: [u8; 4], fill_len: u32, padding: usize) -> PostPadding { - PostPadding { fill, fill_len, padding } + fn new(fill: char, padding: usize) -> PostPadding { + PostPadding { fill, padding } } /// Write this post padding. fn write(self, buf: &mut dyn Write) -> Result { - let fill = unsafe { - str::from_utf8_unchecked(&self.fill.get_unchecked(..self.fill_len as usize)) - }; for _ in 0..self.padding { - buf.write_str(fill)?; + buf.write_char(self.fill)?; } Ok(()) } @@ -1326,20 +1321,11 @@ impl<'a> Formatter<'a> { rt::v1::Alignment::Center => (padding / 2, (padding + 1) / 2), }; - let mut fill = [0; 4]; - let fill_len = { - let fill = self.fill.encode_utf8(&mut fill); - - for _ in 0..pre_pad { - self.buf.write_str(fill)?; - } - - fill.len() - }; + for _ in 0..pre_pad { + self.buf.write_char(self.fill)?; + } - Ok(unsafe { - PostPadding::new(fill, fill_len as u32, post_pad) - }) + Ok(PostPadding::new(self.fill, post_pad)) } /// Takes the formatted parts and applies the padding. diff --git a/src/test/run-make/wasm-stringify-ints-small/Makefile b/src/test/run-make/wasm-stringify-ints-small/Makefile index 18e47242f806b..26de6a0c68990 100644 --- a/src/test/run-make/wasm-stringify-ints-small/Makefile +++ b/src/test/run-make/wasm-stringify-ints-small/Makefile @@ -4,7 +4,7 @@ ifeq ($(TARGET),wasm32-unknown-unknown) all: $(RUSTC) foo.rs -C lto -O --target wasm32-unknown-unknown wc -c < $(TMPDIR)/foo.wasm - [ "`wc -c < $(TMPDIR)/foo.wasm`" -lt "21000" ] + [ "`wc -c < $(TMPDIR)/foo.wasm`" -lt "20500" ] else all: endif From f00f0e676887c8970858897836aa1e8a81d8aa60 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Fri, 8 Feb 2019 10:04:32 +0100 Subject: [PATCH 7/7] Don't shadow the provided `stringify!` macro in a wasm code size test case --- .../run-make/wasm-stringify-ints-small/foo.rs | 40 ++++++++----------- 1 file changed, 17 insertions(+), 23 deletions(-) mode change 100755 => 100644 src/test/run-make/wasm-stringify-ints-small/foo.rs diff --git a/src/test/run-make/wasm-stringify-ints-small/foo.rs b/src/test/run-make/wasm-stringify-ints-small/foo.rs old mode 100755 new mode 100644 index 6aa249aabeb93..7a947f013ad48 --- a/src/test/run-make/wasm-stringify-ints-small/foo.rs +++ b/src/test/run-make/wasm-stringify-ints-small/foo.rs @@ -2,22 +2,14 @@ extern "C" { fn observe(ptr: *const u8, len: usize); - - fn get_u8() -> u8; - fn get_i8() -> i8; - fn get_u16() -> u16; - fn get_i16() -> i16; - fn get_u32() -> u32; - fn get_i32() -> i32; - fn get_u64() -> u64; - fn get_i64() -> i64; - fn get_usize() -> usize; - fn get_isize() -> isize; } -macro_rules! stringify { - ( $($f:ident)* ) => { +macro_rules! s { + ( $( $f:ident -> $t:ty );* $(;)* ) => { $( + extern "C" { + fn $f() -> $t; + } let s = $f().to_string(); observe(s.as_ptr(), s.len()); )* @@ -26,14 +18,16 @@ macro_rules! stringify { #[no_mangle] pub unsafe extern "C" fn foo() { - stringify!(get_u8); - stringify!(get_i8); - stringify!(get_u16); - stringify!(get_i16); - stringify!(get_u32); - stringify!(get_i32); - stringify!(get_u64); - stringify!(get_i64); - stringify!(get_usize); - stringify!(get_isize); + s! { + get_u8 -> u8; + get_i8 -> i8; + get_u16 -> u16; + get_i16 -> i16; + get_u32 -> u32; + get_i32 -> i32; + get_u64 -> u64; + get_i64 -> i64; + get_usize -> usize; + get_isize -> isize; + } }