Reverted inlined lfsr_data_t representation

See previous commit for more details on why this doesn't work:

1. Losing the simple/compiler friendly lfsr_data_t costs more code/stack
   than we save inlined small pieces of data (dids, leb128s, flags,
   etc).

2. Inlined lfsr_data_t is fundamentally incompatible with the new
   lightweight lfsr_rat_t representation for simple data.

Though I did add a comment, and marked lfsr_data_fromslice as inline.

The compiler was apparently already inlining lfsr_data_fromslice (and
it's a valuable optimization!), but making this explicit helps document/
influence future changes.

Code changes:

           code          stack          ctx
  before: 38128           2672          752
  after:  38060 (-0.2%)   2608 (-2.4%)  752 (+0.0%)
This commit is contained in:
Christopher Haster
2025-01-05 16:37:17 -06:00
parent 0b9f46e7cb
commit 0839ac73d6
2 changed files with 58 additions and 107 deletions
+52 -102
View File
@@ -1650,7 +1650,6 @@ static lfs_ssize_t lfsr_bd_progtag(lfs_t *lfs,
/// lfsr_data_t stuff /// /// lfsr_data_t stuff ///
#define LFSR_DATA_ONDISK 0x80000000 #define LFSR_DATA_ONDISK 0x80000000
#define LFSR_DATA_ISIMM 0x40000000
#define LFSR_DATA_NULL() \ #define LFSR_DATA_NULL() \
((lfsr_data_t){ \ ((lfsr_data_t){ \
@@ -1662,15 +1661,6 @@ static lfs_ssize_t lfsr_bd_progtag(lfs_t *lfs,
.size=_size, \ .size=_size, \
.u.buffer=(const void*)(_buffer)}) .u.buffer=(const void*)(_buffer)})
#define LFSR_DATA_IMM(_buffer, _size) \
((struct {lfsr_data_t d;}){lfsr_data_fromimm(_buffer, _size)}.d)
#define LFSR_DATA_LEB128(_word) \
((struct {lfsr_data_t d;}){lfsr_data_fromleb128(_word)}.d)
#define LFSR_DATA_LLEB128(_word) \
((struct {lfsr_data_t d;}){lfsr_data_fromlleb128(_word)}.d)
#define LFSR_DATA_DISK(_block, _off, _size) \ #define LFSR_DATA_DISK(_block, _off, _size) \
((lfsr_data_t){ \ ((lfsr_data_t){ \
.size=LFSR_DATA_ONDISK | (_size), \ .size=LFSR_DATA_ONDISK | (_size), \
@@ -1693,68 +1683,19 @@ static lfs_ssize_t lfsr_bd_progtag(lfs_t *lfs,
.u.disk.off=_off}) .u.disk.off=_off})
#endif #endif
// these can't really be macros, so these get a bit hacky
static inline lfsr_data_t lfsr_data_fromimm(
const void *buffer, lfs_size_t size) {
// inlined-data limited to 8 bytes
LFS_ASSERT(size <= 8);
lfsr_data_t data;
memcpy(data.u.imm, buffer, size);
data.size = LFSR_DATA_ISIMM | size;
return data;
}
#define LFSR_LEB128_DSIZE 5
static inline lfsr_data_t lfsr_data_fromleb128(uint32_t word) {
// leb128s should not exceed 31-bits
LFS_ASSERT(word <= 0x7fffffff);
lfsr_data_t data;
lfs_ssize_t d = lfs_toleb128(word, data.u.imm, LFSR_LEB128_DSIZE);
if (d < 0) {
LFS_UNREACHABLE();
}
data.size = LFSR_DATA_ISIMM | d;
return data;
}
#define LFSR_LLEB128_DSIZE 4
static inline lfsr_data_t lfsr_data_fromlleb128(uint32_t word) {
// little-leb128s should not exceed 28-bits
LFS_ASSERT(word <= 0x0fffffff);
lfsr_data_t data;
lfs_ssize_t d = lfs_toleb128(word, data.u.imm, LFSR_LLEB128_DSIZE);
if (d < 0) {
LFS_UNREACHABLE();
}
data.size = LFSR_DATA_ISIMM | d;
return data;
}
// data helpers // data helpers
static inline bool lfsr_data_ondisk(lfsr_data_t data) { static inline bool lfsr_data_ondisk(lfsr_data_t data) {
return data.size & LFSR_DATA_ONDISK; return data.size & LFSR_DATA_ONDISK;
} }
static inline bool lfsr_data_isbuf(lfsr_data_t data) { static inline bool lfsr_data_isbuf(lfsr_data_t data) {
return (data.size & (LFSR_DATA_ONDISK | LFSR_DATA_ISIMM)) return !(data.size & LFSR_DATA_ONDISK);
== 0;
}
static inline bool lfsr_data_isimm(lfsr_data_t data) {
return (data.size & (LFSR_DATA_ONDISK | LFSR_DATA_ISIMM))
== LFSR_DATA_ISIMM;
} }
static inline lfs_size_t lfsr_data_size(lfsr_data_t data) { static inline lfs_size_t lfsr_data_size(lfsr_data_t data) {
return data.size & ~(LFSR_DATA_ONDISK | LFSR_DATA_ISIMM); return data.size & ~LFSR_DATA_ONDISK;
} }
// fancier data functions
static inline lfsr_data_t lfsr_data_fromslice(lfsr_data_t data, static inline lfsr_data_t lfsr_data_fromslice(lfsr_data_t data,
lfs_ssize_t off, lfs_ssize_t size) { lfs_ssize_t off, lfs_ssize_t size) {
// limit our off/size to data range, note the use of unsigned casts // limit our off/size to data range, note the use of unsigned casts
@@ -1771,18 +1712,10 @@ static inline lfsr_data_t lfsr_data_fromslice(lfsr_data_t data,
data.u.disk.off += off_; data.u.disk.off += off_;
data.size = LFSR_DATA_ONDISK | size_; data.size = LFSR_DATA_ONDISK | size_;
// inlined?
} else if (lfsr_data_isimm(data)) {
memmove(data.u.imm, data.u.imm + off_, size_);
data.size = LFSR_DATA_ISIMM | size_;
// buffer? // buffer?
} else if (lfsr_data_isbuf(data)) { } else {
data.u.buffer += off_; data.u.buffer += off_;
data.size = size_; data.size = size_;
} else {
LFS_UNREACHABLE();
} }
return data; return data;
@@ -1857,10 +1790,6 @@ static lfs_ssize_t lfsr_data_read(lfs_t *lfs, lfsr_data_t *data,
} }
} }
// inlined?
} else if (lfsr_data_isimm(*data)) {
lfs_memcpy(buffer, data->u.imm, d);
// buffer? // buffer?
} else { } else {
lfs_memcpy(buffer, data->u.buffer, d); lfs_memcpy(buffer, data->u.buffer, d);
@@ -1968,26 +1897,14 @@ static lfs_scmp_t lfsr_data_cmp(lfs_t *lfs, lfsr_data_t data,
} }
} }
// inlined?
} else if (lfsr_data_isimm(data)) {
int cmp = lfs_memcmp(data.u.imm, buffer, d);
if (cmp < 0) {
return LFS_CMP_LT;
} else if (cmp > 0) {
return LFS_CMP_GT;
}
// buffer? // buffer?
} else if (lfsr_data_isbuf(data)) { } else {
int cmp = lfs_memcmp(data.u.buffer, buffer, d); int cmp = lfs_memcmp(data.u.buffer, buffer, d);
if (cmp < 0) { if (cmp < 0) {
return LFS_CMP_LT; return LFS_CMP_LT;
} else if (cmp > 0) { } else if (cmp > 0) {
return LFS_CMP_GT; return LFS_CMP_GT;
} }
} else {
LFS_UNREACHABLE();
} }
// if data is equal, check for size mismatch // if data is equal, check for size mismatch
@@ -2050,31 +1967,64 @@ static int lfsr_bd_progdata(lfs_t *lfs,
} }
} }
// inlined?
} else if (lfsr_data_isimm(data)) {
int err = lfsr_bd_prog(lfs, block, off,
data.u.imm, lfsr_data_size(data),
cksum, align);
if (err) {
return err;
}
// buffer? // buffer?
} else if (lfsr_data_isbuf(data)) { } else {
int err = lfsr_bd_prog(lfs, block, off, int err = lfsr_bd_prog(lfs, block, off,
data.u.buffer, lfsr_data_size(data), data.u.buffer, data.size,
cksum, align); cksum, align);
if (err) { if (err) {
return err; return err;
} }
} else {
LFS_UNREACHABLE();
} }
return 0; return 0;
} }
// we can also treat leb128/lleb128 encoding has a high-level operation,
// which is useful for building rats
#define LFSR_LEB128_DSIZE 5
#define LFSR_DATA_LEB128_(_word, _buffer) \
((struct {lfsr_data_t d;}){lfsr_data_fromleb128(_word, _buffer)}.d)
#define LFSR_DATA_LEB128(_word) \
LFSR_DATA_LEB128_(_word, (uint8_t[LFSR_LEB128_DSIZE]){0})
static inline lfsr_data_t lfsr_data_fromleb128(uint32_t word,
uint8_t buffer[static LFSR_LEB128_DSIZE]) {
// leb128s should not exceed 31-bits
LFS_ASSERT(word <= 0x7fffffff);
lfs_ssize_t d = lfs_toleb128(word, buffer, LFSR_LEB128_DSIZE);
if (d < 0) {
LFS_UNREACHABLE();
}
return LFSR_DATA_BUF(buffer, d);
}
#define LFSR_LLEB128_DSIZE 4
#define LFSR_DATA_LLEB128_(_word, _buffer) \
((struct {lfsr_data_t d;}){lfsr_data_fromlleb128(_word, _buffer)}.d)
#define LFSR_DATA_LLEB128(_word) \
LFSR_DATA_LLEB128_(_word, (uint8_t[LFSR_LLEB128_DSIZE]){0})
static inline lfsr_data_t lfsr_data_fromlleb128(uint32_t word,
uint8_t buffer[static LFSR_LLEB128_DSIZE]) {
// little-leb128s should not exceed 28-bits
LFS_ASSERT(word <= 0x0fffffff);
lfs_ssize_t d = lfs_toleb128(word, buffer, LFSR_LLEB128_DSIZE);
if (d < 0) {
LFS_UNREACHABLE();
}
return LFSR_DATA_BUF(buffer, d);
}
// operations on attribute lists // operations on attribute lists
@@ -13481,7 +13431,7 @@ static inline bool lfsr_ocompat_isincompat(lfsr_ocompat_t ocompat) {
// little-endian, truncated bits must be assumed zero // little-endian, truncated bits must be assumed zero
#define LFSR_DATA_COMPAT(_compat) \ #define LFSR_DATA_COMPAT(_compat) \
LFSR_DATA_IMM(((uint8_t[]){ \ LFSR_DATA_BUF(((uint8_t[]){ \
(((_compat) >> 0) & 0xff), \ (((_compat) >> 0) & 0xff), \
(((_compat) >> 8) & 0xff)}), 2) (((_compat) >> 8) & 0xff)}), 2)
+6 -5
View File
@@ -613,15 +613,16 @@ typedef struct lfsr_omdir {
//} lfs_mdir_t; //} lfs_mdir_t;
// either an on-disk or in-device data pointer // either an on-disk or in-device data pointer
//
// note, it's enticing to make this fancier, but we benefit quite a lot
// from the compiler being able to aggresively optimize this struct
//
typedef struct lfsr_data { typedef struct lfsr_data {
// the top bits of size indicate the exact encoding: // sign(size)=0 => in-RAM buffer
// top2(size)=0b00 => in-RAM buffer // sign(size)=1 => on-disk reference
// top2(size)=0b01 => inlined data
// top2(size)=0b1x => on-disk reference
lfs_size_t size; lfs_size_t size;
union { union {
const uint8_t *buffer; const uint8_t *buffer;
uint8_t imm[8];
struct { struct {
lfs_block_t block; lfs_block_t block;
lfs_size_t off; lfs_size_t off;