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 <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
