Re: [PATCH] options, lto: Optimize streaming of optimization nodes

[Richard Biener <rguenther@suse.de>]

On Mon, 14 Sep 2020, Jakub Jelinek wrote:

> On Mon, Sep 14, 2020 at 08:39:22AM +0200, Richard Biener wrote:
> > > When working on the previous patch, I've noticed that all cl_optimization
> > > fields appart from strings are streamed with bp_pack_value (..., 64); so we
> > > waste quite a lot of space, given that many of the options are just booleans
> > > or char options and there are 450-ish of them.
> > > 
> > > Fixed by streaming the number of bits the corresponding fields have.
> > > While for char fields we have also range information, except for 3
> > > it is either -128, 127 or 0, 255, so it didn't seem worth it to bother
> > > with using range-ish packing.
> > > 
> > > Bootstrapped/regtested on x86_64-linux, i686-linux, armv7hl-linux-gnueabi,
> > > aarch64-linux, powerpc64le-linux and lto bootstrapped on x86_64-linux, ok
> > > for trunk?
> > 
> > Hmm, in principle the LTO bytecode format should be not dependent
> > on the host, while it probably doesn't work right now to move
> > 32bit host to 64bit host targeting the same target bytecode files
> > the following makes explicit use of HOST_* and that might have been
> > the reason we're using 64bit encodings for everything.  Note
> > using 64bits isn't too bad in practice since we uleb encode the
> > bitpack words and outputing 64bits basically ensures we're outputting
> > a stream of uleb encoded uint64_t.
> 
> So would using hardcoded 8, 16, 32 and 64 be better?
> I mean, we certainly assume the -128, 127 or 0, 255 ranges e.g. for chars.
> There are ~ 4 strings, 226 chars, 222 ints/enums in my options-save.c
> bp_pack_* calls and 39 64-bit (which includes the target stuff) on
> x86_64-linux.
>
> Another option is using the variable length
> bp_pack_var_len_unsigned/bp_pack_var_len_int
> for everything, I guess most of the char options are 0/1/2, most of the
> params are smallish too (though there are some 100s, 1000s and 10000s and
> even higher, but most of them are small).
> Either the awk script would need to figure out which are guaranteed to be
> unsigned and use *_unsigned for them and use *_int for rest, or we'd need to
> use *_int everywhere.

But does it make any noticable difference in the end?  Using
bp_pack_var_len_unsigned just causes us to [u]leb encode half-bytes
rather than full bytes.  Using hardcoded 8/16/32/64 makes it still
dependent on what 'int' is at maximum on the host.

That is, I'd indeed prefer bp_pack_var_len_unsigned over hard-coding
8, 16, etc., but can you share a size comparison of the bitpack?
I guess with bp_pack_var_len_unsigned it might shrink in half
compared to the current code and streaming standard -O2?

Richard.
 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)