Github/littlefs
1
Fork 0
Code Actions Activity

Attempting to enable asserts-as-hints with isopen as an exception

Browse Source 
The theory is that lfsr_opened_isopen is a relatively special case, and
the main cause of LFS_ASSERT side-effects. All other LFS_ASSERTs are
either limited to simple expressions, or small static-inline functions.

Unfortunately, even without lfsr_opened_isopen asserts, it seems
asserts-as-hints still results in worse code/stack costs:

                        code          stack
  no-assert (before):  33626           2552
  hint-assert (after): 33670 (+0.1%)   2592 (+1.6%)

Digging around in the low-level assembly, it seems that what is
happening is the increased number of calls to static-inline functions is
causing the compiler to prefer to not-inline functions more often. Then,
even if calls would be eliminated as dead-code, the damage is done to
the containing function.

It's not entirely clear how this could be avoided. Maybe a separate
LFS_ASSERT for only pure expressions? This may not be worth trying to
solve outside of the compiler...
This commit is contained in:
Christopher Haster
2024-06-10 00:40:49 -05:00
parent b563050fc8
commit 9c2c101e7f
2 changed files with 68 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+66 -27
View File
@@ -1140,8 +1140,8 @@ static lfs_ssize_t lfsr_bd_readtag(lfs_t *lfs,
}
int err = lfsr_bd_read(lfs, block, off, hint, tag_buf, tag_dsize);
LFS_ASSERT(err <= 0);
if (err < 0) {
if (err) {
LFS_ASSERT(err < 0);
return err;
}
@@ -1232,8 +1232,8 @@ static lfs_ssize_t lfsr_bd_progtag(lfs_t *lfs,
int err = lfsr_bd_prog(lfs, block, off, tag_buf, d,
cksum_, align);
LFS_ASSERT(err <= 0);
if (err < 0) {
if (err) {
LFS_ASSERT(err < 0);
return err;
}
@@ -1328,8 +1328,8 @@ static lfs_ssize_t lfsr_data_read(lfs_t *lfs, lfsr_data_t *data,
// note our hint includes the full data range
lfsr_data_size(*data),
buffer, d);
LFS_ASSERT(err <= 0);
if (err < 0) {
if (err) {
LFS_ASSERT(err < 0);
return err;
}
@@ -1443,8 +1443,8 @@ static lfs_scmp_t lfsr_data_namecmp(lfs_t *lfs, lfsr_data_t data,
// first compare the did
lfsr_did_t did_;
int err = lfsr_data_readleb128(lfs, &data, &did_);
LFS_ASSERT(err <= 0);
if (err < 0) {
if (err) {
LFS_ASSERT(err < 0);
return err;
}
@@ -2352,8 +2352,6 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
static int lfsr_rbyd_fetchvalidate(lfs_t *lfs, lfsr_rbyd_t *rbyd,
lfs_block_t block, lfs_size_t trunk, lfsr_rid_t weight,
uint32_t cksum) {
(void)weight;
int err = lfsr_rbyd_fetch(lfs, rbyd, block, trunk);
if (err) {
if (err == LFS_ERR_CORRUPT) {
@@ -3584,11 +3582,11 @@ static lfs_ssize_t lfsr_rbyd_estimate(lfs_t *lfs, const lfsr_rbyd_t *rbyd,
int err = lfsr_rbyd_lookupnext(lfs, rbyd,
a_rid, tag+1,
&rid_, &tag, &weight_, &data);
LFS_ASSERT(err <= 0);
if (err < 0) {
if (err) {
if (err == LFS_ERR_NOENT) {
break;
}
LFS_ASSERT(err < 0);
return err;
}
if (rid_ > a_rid+lfs_smax32(weight_-1, 0)) {
@@ -3886,9 +3884,9 @@ static lfs_scmp_t lfsr_rbyd_namelookup(lfs_t *lfs, const lfsr_rbyd_t *rbyd,
// of a weighted rid with this
lower_rid + (upper_rid-1-lower_rid)/2, 0,
&rid__, &tag__, &weight__, &data__);
LFS_ASSERT(err <= 0);
if (err < 0) {
if (err) {
LFS_ASSERT(err != LFS_ERR_NOENT);
LFS_ASSERT(err < 0);
return err;
}
@@ -4891,9 +4889,9 @@ static lfs_scmp_t lfsr_btree_namelookup(lfs_t *lfs, const lfsr_btree_t *btree,
lfsr_data_t data__;
int err = lfsr_rbyd_sublookup(lfs, &branch, rid__, LFSR_TAG_STRUCT,
&tag__, &data__);
LFS_ASSERT(err <= 0);
if (err < 0) {
if (err) {
LFS_ASSERT(err != LFS_ERR_NOENT);
LFS_ASSERT(err < 0);
return err;
}
@@ -4904,8 +4902,8 @@ static lfs_scmp_t lfsr_btree_namelookup(lfs_t *lfs, const lfsr_btree_t *btree,
// fetch the next branch
err = lfsr_data_readbranch(lfs, &data__, weight__, &branch);
LFS_ASSERT(err <= 0);
if (err < 0) {
if (err) {
LFS_ASSERT(err < 0);
return err;
}
@@ -5152,13 +5150,17 @@ static bool lfsr_opened_isopen(lfs_t *lfs, const lfsr_opened_t *o) {
}
static void lfsr_opened_add(lfs_t *lfs, lfsr_opened_t *o) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(!lfsr_opened_isopen(lfs, o));
#endif
o->next = lfs->opened;
lfs->opened = o;
}
static void lfsr_opened_remove(lfs_t *lfs, lfsr_opened_t *o) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, o));
#endif
for (lfsr_opened_t **o_ = &lfs->opened; *o_; o_ = &(*o_)->next) {
if (*o_ == o) {
*o_ = (*o_)->next;
@@ -5583,7 +5585,6 @@ static void lfsr_fs_revertgdelta(lfs_t *lfs) {
&LFSR_DATA_BUF(lfs->grm_p, LFSR_GRM_DSIZE),
&lfs->grm);
LFS_ASSERT(!err);
(void)err;
}
static void lfsr_fs_commitgdelta(lfs_t *lfs) {
@@ -6362,11 +6363,11 @@ static lfs_ssize_t lfsr_mdir_estimate__(lfs_t *lfs, const lfsr_mdir_t *mdir,
int err = lfsr_rbyd_lookupnext(lfs, &mdir->rbyd,
a_rid, tag+1,
&rid_, &tag, NULL, &data);
LFS_ASSERT(err <= 0);
if (err < 0) {
if (err) {
if (err == LFS_ERR_NOENT) {
break;
}
LFS_ASSERT(err < 0);
return err;
}
if (rid_ != a_rid) {
@@ -6393,8 +6394,8 @@ static lfs_ssize_t lfsr_mdir_estimate__(lfs_t *lfs, const lfsr_mdir_t *mdir,
lfsr_shrub_t shrub;
err = lfsr_data_readshrub(lfs, &data, mdir, &shrub);
LFS_ASSERT(err <= 0);
if (err < 0) {
if (err) {
LFS_ASSERT(err < 0);
return err;
}
@@ -9689,8 +9690,10 @@ int lfsr_stat(lfs_t *lfs, const char *path, struct lfs_info *info) {
static int lfsr_dir_rewind_(lfs_t *lfs, lfsr_dir_t *dir);
int lfsr_dir_open(lfs_t *lfs, lfsr_dir_t *dir, const char *path) {
#ifndef LFS_NO_ASSERT
// already open?
LFS_ASSERT(!lfsr_opened_isopen(lfs, &dir->o));
#endif
// setup dir state
dir->o.type = LFS_TYPE_DIR;
@@ -9746,7 +9749,9 @@ int lfsr_dir_open(lfs_t *lfs, lfsr_dir_t *dir, const char *path) {
}
int lfsr_dir_close(lfs_t *lfs, lfsr_dir_t *dir) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &dir->o));
#endif
// remove from tracked mdirs
lfsr_opened_remove(lfs, &dir->o);
@@ -9754,7 +9759,9 @@ int lfsr_dir_close(lfs_t *lfs, lfsr_dir_t *dir) {
}
int lfsr_dir_read(lfs_t *lfs, lfsr_dir_t *dir, struct lfs_info *info) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &dir->o));
#endif
// was our dir removed?
if (lfsr_f_iszombie(dir->o.flags)) {
@@ -9828,7 +9835,9 @@ int lfsr_dir_read(lfs_t *lfs, lfsr_dir_t *dir, struct lfs_info *info) {
}
int lfsr_dir_seek(lfs_t *lfs, lfsr_dir_t *dir, lfs_soff_t off) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &dir->o));
#endif
// do nothing if removed
if (lfsr_f_iszombie(dir->o.flags)) {
@@ -9858,7 +9867,9 @@ int lfsr_dir_seek(lfs_t *lfs, lfsr_dir_t *dir, lfs_soff_t off) {
lfs_soff_t lfsr_dir_tell(lfs_t *lfs, lfsr_dir_t *dir) {
(void)lfs;
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &dir->o));
#endif
return dir->pos;
}
@@ -9891,7 +9902,9 @@ static int lfsr_dir_rewind_(lfs_t *lfs, lfsr_dir_t *dir) {
}
int lfsr_dir_rewind(lfs_t *lfs, lfsr_dir_t *dir) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &dir->o));
#endif
return lfsr_dir_rewind_(lfs, dir);
}
@@ -10038,8 +10051,10 @@ static lfs_ssize_t lfsr_bshrub_read(lfs_t *lfs, const lfsr_file_t *file,
int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file,
const char *path, uint32_t flags,
const struct lfs_file_config *cfg) {
#ifndef LFS_NO_ASSERT
// already open?
LFS_ASSERT(!lfsr_opened_isopen(lfs, &file->o));
#endif
// don't allow the forbidden mode!
LFS_ASSERT((flags & 3) != 3);
// these flags require a writable file
@@ -10230,7 +10245,9 @@ int lfsr_file_open(lfs_t *lfs, lfsr_file_t *file,
int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file);
int lfsr_file_close(lfs_t *lfs, lfsr_file_t *file) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &file->o));
#endif
// don't call lfsr_file_sync if we're readonly or desynced
int err = 0;
@@ -10282,8 +10299,8 @@ static lfs_ssize_t lfsr_bshrub_estimate(lfs_t *lfs, const lfsr_file_t *file) {
lfsr_data_t data;
int err = lfsr_mdir_lookupnext(lfs, &file->o.mdir, LFSR_TAG_DATA,
&tag, &data);
LFS_ASSERT(err <= 0);
if (err < 0 && err != LFS_ERR_NOENT) {
if (err && err != LFS_ERR_NOENT) {
LFS_ASSERT(err < 0);
return err;
}
@@ -10298,8 +10315,8 @@ static lfs_ssize_t lfsr_bshrub_estimate(lfs_t *lfs, const lfsr_file_t *file) {
lfsr_shrub_t shrub;
err = lfsr_data_readshrub(lfs, &data, &file->o.mdir,
&shrub);
LFS_ASSERT(err <= 0);
if (err < 0) {
if (err) {
LFS_ASSERT(err < 0);
return err;
}
@@ -11452,7 +11469,9 @@ fragment:;
lfs_ssize_t lfsr_file_read(lfs_t *lfs, lfsr_file_t *file,
void *buffer, lfs_size_t size) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &file->o));
#endif
// can't read from writeonly files
LFS_ASSERT(!lfsr_o_iswronly(file->o.flags));
LFS_ASSERT(file->pos + size <= 0x7fffffff);
@@ -11544,7 +11563,9 @@ lfs_ssize_t lfsr_file_read(lfs_t *lfs, lfsr_file_t *file,
lfs_ssize_t lfsr_file_write(lfs_t *lfs, lfsr_file_t *file,
const void *buffer, lfs_size_t size) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &file->o));
#endif
// can't write to readonly files
LFS_ASSERT(!lfsr_o_isrdonly(file->o.flags));
@@ -11699,7 +11720,9 @@ failed:;
}
int lfsr_file_flush(lfs_t *lfs, lfsr_file_t *file) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &file->o));
#endif
// readonly files should do nothing
LFS_ASSERT(!lfsr_o_isrdonly(file->o.flags)
|| !lfsr_f_isunflush(file->o.flags)
@@ -11745,7 +11768,9 @@ failed:;
}
int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &file->o));
#endif
// removed? we can't sync
if (lfsr_f_iszombie(file->o.flags)) {
return LFS_ERR_NOENT;
@@ -11911,7 +11936,9 @@ failed:;
int lfsr_file_desync(lfs_t *lfs, lfsr_file_t *file) {
(void)lfs;
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &file->o));
#endif
file->o.flags |= LFS_O_DESYNC;
return 0;
@@ -11919,7 +11946,9 @@ int lfsr_file_desync(lfs_t *lfs, lfsr_file_t *file) {
lfs_soff_t lfsr_file_seek(lfs_t *lfs, lfsr_file_t *file,
lfs_soff_t off, uint8_t whence) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &file->o));
#endif
// TODO check for out-of-range?
@@ -11947,14 +11976,18 @@ lfs_soff_t lfsr_file_seek(lfs_t *lfs, lfsr_file_t *file,
lfs_soff_t lfsr_file_tell(lfs_t *lfs, lfsr_file_t *file) {
(void)lfs;
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &file->o));
#endif
return file->pos;
}
lfs_soff_t lfsr_file_rewind(lfs_t *lfs, lfsr_file_t *file) {
(void)lfs;
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &file->o));
#endif
file->pos = 0;
return 0;
@@ -11962,13 +11995,17 @@ lfs_soff_t lfsr_file_rewind(lfs_t *lfs, lfsr_file_t *file) {
lfs_soff_t lfsr_file_size(lfs_t *lfs, lfsr_file_t *file) {
(void)lfs;
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &file->o));
#endif
return lfsr_file_size_(file);
}
int lfsr_file_truncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size_) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &file->o));
#endif
// exceeds our file limit?
if (size_ > lfs->file_limit) {
@@ -12073,7 +12110,9 @@ failed:;
}
int lfsr_file_fruncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size_) {
#ifndef LFS_NO_ASSERT
LFS_ASSERT(lfsr_opened_isopen(lfs, &file->o));
#endif
// exceeds our file limit?
if (size_ > lfs->file_limit) {
lfs_util.h
+2
View File
@@ -96,6 +96,8 @@ extern "C"
#ifndef LFS_ASSERT
#ifndef LFS_NO_ASSERT
#define LFS_ASSERT(test) assert(test)
#elif !defined(LFS_NO_BUILTINS)
#define LFS_ASSERT(test) ((test) ? (void)0 : __builtin_unreachable())
#else
#define LFS_ASSERT(test)
#endif