From 77c45827e52162d61f16bafa9500965e85fdc0d8 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 15 Apr 2024 01:45:22 -0500 Subject: [PATCH] rbyd-rr: Explicitly deduplicated diverging conditions I'm not really sure why the compiler isn't taking care of this for us. Usually I prefer duplicated logic over more variables since it means less state to keep track of when reading/debugging, and the compiler will optimize it away anyways. But I guess these conditions are just too complicated in this case? Maybe the compiler is trying to take advantage of &&/|| short-circuiting even with -Os? Even marking the lfsr_tag_diverging* functions with __attribute__((noinline, pure, const)) doesn't help... Oh well, this is a case where we can just make the deduplication explicit for a bit of code savings: code stack before: 34176 2864 after: 34080 (-0.3%) 2864 (+0.0%) code frame stack appendattr before: 2162 208 560 appendattr after: 2104 (-2.7%) 216 (+3.8%) 568 (+1.4%) --- lfs.c | 51 +++++++++++++++++---------------------------------- 1 file changed, 17 insertions(+), 34 deletions(-) diff --git a/lfs.c b/lfs.c index 96be579b..964ddf3a 100644 --- a/lfs.c +++ b/lfs.c @@ -2935,38 +2935,28 @@ trunk:; } // do bounds want to take different paths? begin diverging + bool diverging = lfsr_tag_diverging2( + alt, weight, + p[0].alt, p[0].weight, + lower_rid, upper_rid, + a_rid, a_tag, + b_rid, b_tag); + bool diverging_red = lfsr_tag_isred(p[0].alt) + && lfsr_tag_diverging( + p[0].alt, p[0].weight, + lower_rid, upper_rid, + a_rid, a_tag, + b_rid, b_tag); if (!diverged // diverging black? && (lfsr_tag_isblack(alt) // give up if we find a yellow alt || lfsr_tag_isred(p[0].alt)) - && (lfsr_tag_diverging2( - alt, weight, - p[0].alt, p[0].weight, - lower_rid, upper_rid, - a_rid, a_tag, - b_rid, b_tag) - || (lfsr_tag_isred(p[0].alt) - && lfsr_tag_diverging( - p[0].alt, p[0].weight, - lower_rid, upper_rid, - a_rid, a_tag, - b_rid, b_tag)))) { + && (diverging || diverging_red)) { diverged = true; - // both diverged? collapse - if (lfsr_tag_diverging2( - alt, weight, - p[0].alt, p[0].weight, - lower_rid, upper_rid, - a_rid, a_tag, - b_rid, b_tag) - && (lfsr_tag_isred(p[0].alt) - && lfsr_tag_diverging( - p[0].alt, p[0].weight, - lower_rid, upper_rid, - a_rid, a_tag, - b_rid, b_tag))) { + // both diverging? collapse + if (diverging && diverging_red) { LFS_ASSERT(a_rid < b_rid || a_tag < b_tag); LFS_ASSERT(lfsr_tag_isparallel(alt, p[0].alt)); @@ -2997,15 +2987,8 @@ trunk:; continue; } - // force diverged alts to be pruned - } else if (diverged - && lfsr_tag_diverging2( - alt, weight, - p[0].alt, p[0].weight, - lower_rid, upper_rid, - a_rid, a_tag, - b_rid, b_tag)) { - // one diverged? trim so alt is pruned + // trim diverging alts so they can be pruned + } else if (diverged && diverging) { lfsr_tag_trim( alt, weight, &lower_rid, &upper_rid,