From dcc67d22a8cdec98b9c37def4daf5150fe9e2fa7 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 30 Mar 2024 02:13:22 -0500 Subject: [PATCH] rbyd-rr: Tweaked lfsr_tag_follow to make altn/alta implicit again In theory, checking altn/alta tags for followability should be implicit. These are encoding as altle/altgt tag 0, which should never be requested in normal operation: altn => altle 0 alta => altgt 0 But while that's good in theory, null tags, tag 0, has a tendency to creep into these functions and has already caused a number of headaches. Conditionally checking for altn/alta is safer, but asserting on tag 0 is just as safe and adds no code cost. Both lfsr_rbyd_appendattr and lfsr_rbyd_lookupnext have `tag = lfs_max16(tag, 0x1)` guards now to comply with this rule. But it's still a nice safety net to assert on tag 0 in lfsr_tag_follow*. In case you were curious if the max16 guards were more expensive than the explicit altn/alta checks, code size says no: code stack before: 34256 2864 after: 34220 (-0.1%) 2864 (+0.0%) --- lfs.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lfs.c b/lfs.c index ee06b22c..3a8a9cfd 100644 --- a/lfs.c +++ b/lfs.c @@ -889,16 +889,17 @@ static inline bool lfsr_tag_follow( lfsr_tag_t alt, lfsr_rid_t weight, lfsr_srid_t lower_rid, lfsr_srid_t upper_rid, lfsr_srid_t rid, lfsr_tag_t tag) { + // null tags break the following logic for altns/altas + LFS_ASSERT(lfsr_tag_key(tag) != 0); + if (lfsr_tag_isgt(alt)) { return rid > upper_rid - (lfsr_srid_t)weight - 1 || (rid == upper_rid - (lfsr_srid_t)weight - 1 - && (lfsr_tag_isa(alt) - || lfsr_tag_key(tag) > lfsr_tag_key(alt))); + && lfsr_tag_key(tag) > lfsr_tag_key(alt)); } else { return rid < lower_rid + (lfsr_srid_t)weight - 1 || (rid == lower_rid + (lfsr_srid_t)weight - 1 - && (!lfsr_tag_isn(alt) - && lfsr_tag_key(tag) <= lfsr_tag_key(alt))); + && lfsr_tag_key(tag) <= lfsr_tag_key(alt)); } } @@ -2826,7 +2827,7 @@ static int lfsr_rbyd_appendattr(lfs_t *lfs, lfsr_rbyd_t *rbyd, } else if (lfsr_tag_issub(tag)) { a_tag = lfsr_tag_supkey(tag); b_tag = lfsr_tag_supkey(tag) + 0x100; - } else if (lfsr_tag_isrm(tag) || !lfsr_tag_key(tag)) { + } else if (lfsr_tag_isrm(tag)) { a_tag = lfsr_tag_key(tag); b_tag = lfsr_tag_key(tag) + 1; } else { @@ -2834,6 +2835,8 @@ static int lfsr_rbyd_appendattr(lfs_t *lfs, lfsr_rbyd_t *rbyd, b_tag = lfsr_tag_key(tag); } } + a_tag = lfs_max16(a_tag, 0x1); + b_tag = lfs_max16(b_tag, 0x1); // keep track of diverged state //