From 036047bbba85fc988702c45c08b0ee822ef159c4 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 10 Feb 2024 21:08:48 -0600 Subject: [PATCH] Reverted little-leb128 decoder to just call the big-leb128 decoder The duplicate decoder for little-leb128 avoided extra stack allocation for the unaligned worst-case leb128 encoding, but did result in a duplicate function and extra code cost. Reasons for deduplicating: - We'd definitely want to deduplicate these functions if they end up with the same encoding cost (28-bit littlefs mode?). - Less code is less code. - I noticed the stack savings are arch dependent because lfsr_data_readlleb128 only sometimes ends up on the "hot-path". thumb calls lfsr_data_readlleb128 on the hot-path, but x86 ends up in lfsr_bd_readtag. So it's not clear this stack savings is really valuable vs buffer reductions higher up the stack. Though I'm not really sure how much I trust stack.py based analysis right now... - 8 bytes of RAM is more likely to be compiler noise than 100 bytes of code. Still, both are somewhat negligible and I should probably move on from this... I did also try an internally deduplicated version, with an lfsr_data_readleb128_ that takes a buffer provided by both lfsr_data_readleb128 and lfsr_data_readlleb128, but this ended up the worst of both worlds likely just due to compiler overhead. Abstractions have cost! code stack duplicated: 33808 2792 little-calls-big: 33700 (-0.3%) 2800 (+0.3%) dedup-via-buffer: 33796 (-0.0%) 2816 (+0.9%) --- lfs.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/lfs.c b/lfs.c index f2996219..22d6c683 100644 --- a/lfs.c +++ b/lfs.c @@ -1273,25 +1273,14 @@ static int lfsr_data_readleb128(lfs_t *lfs, lfsr_data_t *data, // resulting leb128 encoding fits nicely in 4-bytes static inline int lfsr_data_readlleb128(lfs_t *lfs, lfsr_data_t *data, uint32_t *word_) { - // note we make sure not to update our data offset until after leb128 - // decoding - lfsr_data_t data_ = *data; - - // for 28-bits we can assume worst-case leb128 size is 4-bytes - uint8_t buf[4]; - lfs_ssize_t d = lfsr_data_read(lfs, &data_, buf, 4); - if (d < 0) { - return d; + // just call readleb128 here + int err = lfsr_data_readleb128(lfs, data, word_); + if (err) { + return err; } - d = lfs_fromleb128(word_, buf, d); - if (d < 0) { - return d; - } // little-leb128s should be limited to 28-bits LFS_ASSERT(*word_ <= 0x0fffffff); - - *data = lfsr_data_slice(*data, d, -1); return 0; }