rbyd-rr: Simplified diverging state machine, rely on relative a/b ordering

So instead of explicitly keeping track of which bound we are on, either via
separate DIVERGEDLOWER/DIVERGEDUPPER states or a d_upper bool, we can
infer the bound based on the relative ordering a_rid/tag and b_rid/tag:

- a_rid < b_rid || a_tag < b_tag   => lower bound
- a_rid > b_rid || a_tag > b_tag   => upper bound
- a_rid == b_rid && a_tag == b_tag => not diverging

This is more appealing now that we don't rely on the specific bound for
diverged triming. The only remaining state is if we have diverged yet, a
simple boolean.

Measuring code size was a bit confusing. During a partial edit, it
looked like this was going to save a bit of code, but the result was
actually worse. It seems that explicitly masking/oring a single bit in
the original uint8_t d_state is somehow cheaper than storing if we have
diverged as a bool?

            code          stack
  before:  34516           2864
  bitmask: 34504 (-0.0%)   2864 (+0.0%)
  boolean: 34528 (+0.0%)   2864 (+0.0%)

                      code          frame         stack
  appendattr before:  2366           216            568
  appendattr bitmask: 2354 (-0.5%)   216 (+0.0%)    568 (+0.0%)
  appendattr boolean: 2378 (+0.5%)   216 (+0.0%)    568 (+0.0%)

No idea why this would happen. If feels like some sort of
compiler/optimizer bug... But this is pretty close to the compiler noise
floor and compilers aren't perfect. I'm probably reading too much into
an extra 24 bytes...

This is still a worthwhile change as it's usually good to prefer
implicit state over explicit. Less things can fall out of sync this way.
This commit is contained in:
Christopher Haster
2024-04-08 20:35:51 -05:00
parent ffc36b0f36
commit f06ef46e8b
+12 -33
View File
@@ -2785,30 +2785,6 @@ static void lfsr_p_recolor(
} }
} }
// diverged state machine for range appends
enum {
LFSR_D_NOTDIVERGEDLOWER = 0x0,
LFSR_D_NOTDIVERGEDUPPER = 0x1,
LFSR_D_DIVERGEDLOWER = 0x2,
LFSR_D_DIVERGEDUPPER = 0x3,
};
static inline bool lfsr_d_isdiverged(uint8_t d_state) {
return d_state & LFSR_D_DIVERGEDLOWER;
}
static inline bool lfsr_d_isupper(uint8_t d_state) {
return d_state & LFSR_D_NOTDIVERGEDUPPER;
}
static inline bool lfsr_d_islower(uint8_t d_state) {
return !lfsr_d_isupper(d_state);
}
static inline uint8_t lfsr_d_diverge(uint8_t d_state) {
return d_state |= LFSR_D_DIVERGEDLOWER;
}
// core rbyd algorithm // core rbyd algorithm
static int lfsr_rbyd_appendattr(lfs_t *lfs, lfsr_rbyd_t *rbyd, static int lfsr_rbyd_appendattr(lfs_t *lfs, lfsr_rbyd_t *rbyd,
lfsr_srid_t rid, lfsr_tag_t tag, lfsr_srid_t delta, lfsr_data_t data) { lfsr_srid_t rid, lfsr_tag_t tag, lfsr_srid_t delta, lfsr_data_t data) {
@@ -2895,7 +2871,7 @@ static int lfsr_rbyd_appendattr(lfs_t *lfs, lfsr_rbyd_t *rbyd,
// 2. to write the common trunk + diverged-upper trunk, stitching the // 2. to write the common trunk + diverged-upper trunk, stitching the
// two diverged trunks together where they diverged // two diverged trunks together where they diverged
// //
uint8_t d_state = LFSR_D_NOTDIVERGEDLOWER; bool diverged = false;
lfsr_srid_t d_rid = 0; lfsr_srid_t d_rid = 0;
lfsr_tag_t d_tag = 0; lfsr_tag_t d_tag = 0;
@@ -2953,7 +2929,7 @@ trunk:;
lfs_size_t branch_ = branch + d; lfs_size_t branch_ = branch + d;
// do bounds want to take different paths? begin diverging // do bounds want to take different paths? begin diverging
if (!lfsr_d_isdiverged(d_state) if (!diverged
// diverging black? // diverging black?
&& (((lfsr_tag_isblack(alt) && (((lfsr_tag_isblack(alt)
// give up if we find a yellow alt // give up if we find a yellow alt
@@ -2971,7 +2947,7 @@ trunk:;
lower_rid, upper_rid, lower_rid, upper_rid,
a_rid, a_tag, a_rid, a_tag,
b_rid, b_tag)))) { b_rid, b_tag)))) {
d_state = lfsr_d_diverge(d_state); diverged = true;
// diverging red? flip // diverging red? flip
if (lfsr_tag_isred(p[0].alt) if (lfsr_tag_isred(p[0].alt)
@@ -3012,7 +2988,7 @@ trunk:;
} }
// diverging upper? stitch together both trunks // diverging upper? stitch together both trunks
if (lfsr_d_isupper(d_state)) { if (a_rid > b_rid || a_tag > b_tag) {
if (lfsr_tag_isgt(alt)) { if (lfsr_tag_isgt(alt)) {
lfsr_tag_flip2( lfsr_tag_flip2(
&alt, &weight, &alt, &weight,
@@ -3041,7 +3017,7 @@ trunk:;
} }
// force diverged alts to be pruned // force diverged alts to be pruned
} else if (lfsr_d_isdiverged(d_state) } else if (diverged
&& lfsr_tag_diverging2( && lfsr_tag_diverging2(
alt, weight, alt, weight,
p[0].alt, p[0].weight, p[0].alt, p[0].weight,
@@ -3245,9 +3221,9 @@ trunk:;
// the last alt should always end up black // the last alt should always end up black
LFS_ASSERT(lfsr_tag_isblack(p[0].alt)); LFS_ASSERT(lfsr_tag_isblack(p[0].alt));
if (diverged) {
// diverged lower trunk? move on to upper trunk // diverged lower trunk? move on to upper trunk
if (d_state == LFSR_D_DIVERGEDLOWER) { if (a_rid < b_rid || a_tag < b_tag) {
d_state = LFSR_D_NOTDIVERGEDUPPER;
// keep track of the lower diverged bound // keep track of the lower diverged bound
d_rid = lower_rid; d_rid = lower_rid;
d_tag = lower_tag; d_tag = lower_tag;
@@ -3269,16 +3245,19 @@ trunk:;
} }
// swap tag/rid and move on to upper trunk // swap tag/rid and move on to upper trunk
diverged = false;
branch = trunk_; branch = trunk_;
lfs_swap16(&a_tag, &b_tag); lfs_swap16(&a_tag, &b_tag);
lfs_sswap32(&a_rid, &b_rid); lfs_sswap32(&a_rid, &b_rid);
goto trunk; goto trunk;
} else if (d_state == LFSR_D_DIVERGEDUPPER) { } else {
// use the lower diverged bound for leaf weight calculation // use the lower diverged bound for leaf weight
// calculation
lower_rid = d_rid; lower_rid = d_rid;
lower_tag = d_tag; lower_tag = d_tag;
} }
}
goto stem; goto stem;
} }