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%)
This commit is contained in:
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user