[AUTOMATED] feat(p8): loopcondhoist - defer a loop head in the ifNoExit scan so ruleBlockWhileDo keeps the head test (+669 GED-perfect / 32,339 O0 functions)

[mahaloz]

Stacked on #281 (feat/guardarm) — same 60 lines, same new module, and the
catalog counts collide. Merge order is at the bottom of this body; read it
before merging #281, because delete_branch_on_merge is set repo-wide and
merging #281 would delete feat/guardarm and auto-close this PR.

Closes symptom B of the round-4 ifnoexit cluster
(docs/decbench/features-round4.md Rank 2). The filed root cause there is
refuted (Killed K7) and the filed "ship form A first" plan is refuted
(Killed K8); neither is what this ships.

Phase 1 — the decision trace, and its verdict

Three independent refuters asked for one experiment: a decision trace inside
CollapseStructure::rule_block_if_no_exit. It ships in this PR
(p8_structure/kuna_ifnoexit.rs + the KUNA_RS_DEBUG prints in
blockaction.rs), and the verdict is the deliverable, independent of the two
fixes. Reproduce with
KUNA_RS_DEBUG=1 kuna decompile-all <bin> --addr 0x... — note kuna decompile
spawns decomp_dbg with piped stderr and swallows the trace, so decompile-all
is the surface that shows it.

Per candidate it prints bl's type / degree / start address, both arms'
size_in, size_out, is_decision_out and exit-leaf halt type (testing the
pcodeop_flags::noreturn bit, 0x1000000, not the return(#0x1:4) shape —
badinstruction/unimplemented/missing/halt are identical in shape),
whether bl is a live loop head, whether rule_block_while_do would fire right
now, the whole deferred-scan order, and the chosen i. The apply#N counter is
in every header because collapse_all runs twice per function.

1. Is A an arm predicate, a scan-order predicate, or both? — a pure arm
predicate, and only in a tie. On dpkg-query control_list both guards are
in=1 out=0 dec=true on both arms, the rule takes i=0 in both, and the only
difference between the guard that is right and the guard that is wrong is which
side the halt happened to land on:

[ifne] === func=sub_6c1c entry=0x6c1c apply#0 ===
[ifne] cand bl=#2@0x6c7c:Copy in=1 out=2 loophead=no whiledo_would=no
[ifne]   arm0 #4@0x6cab:Copy in=1 out=0 dec=true halt=noreturn
[ifne]   arm1 #3@0x6cde:Copy in=1 out=0 dec=true halt=-
[ifne]   CHOSE i=0                                    <- CORRECT (halt on out(0))
[ifne] cand bl=#0@0x6c20:Condition in=0 out=2 loophead=no whiledo_would=no
[ifne]   arm0 #2@0x6c7c:Ls   in=1 out=0 dec=true halt=-
[ifne]   arm1 #5@0x6c53:Copy in=1 out=0 dec=true halt=noreturn
[ifne]   CHOSE i=0                                    <- INVERTED (halt on out(1))

When only one arm is eligible the for i in 0..2 loop already picks it, which is
why ~84% of the filed signature population is already right and is untouched by
any tie-break. The scan order is irrelevant to A: the same block is the only
candidate either way.

Why out-index carries no source information. The trace also settles the
"filed root cause predicts the opposite of the output" contradiction the refuters
logged on scp xcalloc (objdump puts the fatal block on out(0), yet kuna
emits the inverse). apply#0 picks the halt arm at i=0 correctly — and
that pick negates the condition, take_pending_flips ->
block_basic_negate_lastop -> swap_edges re-orients the block, and apply#1
sees the arms swapped and picks the return. The second run undoes the first.
The orientation at the rule is not the disassembly orientation.

2. Is B the same site, and does fixing the scan order alone fix A? — same
60 lines, different predicate, and no. On coreutils od read_char and
libacl getfacl walk_tree_visited the trace shows loophead=yes,
whiledo_would=no, and exactly one eligible arm — the tie-break never runs,
so nothing an arm predicate does can touch B. Conversely the guards in A are
loophead=no and are the only candidate in their scan, so nothing a scan-order
predicate does can touch A. They are disjoint on every witness.

[ifne] --- deferred scan (apply#1) size=6 order=[#0 out=1  #1@0x5395 out=2  #2@0x5340 out=2  #3@0x53a4 out=0  #4 out=1  #5@0x53a4 out=0]
[ifne] cand bl=#1@0x5395:Copy in=2 out=2 loophead=YES(depth=0) whiledo_would=no
[ifne]   arm0 #5@0x53a4:Copy in=1 out=0 dec=true halt=-     <- the `return v3` exit
[ifne]   arm1 #2@0x5340:Copy in=1 out=2 dec=true halt=-     <- the loop body
[ifne]   CHOSE i=0

Two further facts fall out of that dump. The better candidate is already in
the same scan, one position behind: #2@0x5340, the block carrying the
source's break, is itself ifNoExit-eligible (its arm #3@0x53a4 is
in=1 out=0) — first-match in component order simply reaches the head first.
And the rule only becomes eligible on the head after the return split: in
apply#0 the shared return v3 block is in=2, the rule declines, and nothing
fires; ActionReturnSplit/returndup then gives it in-degree 1 and apply#1
folds the head. That is why B is an -O0 population, and it is also why the filed
region_structurer.rs:1788 cb.size_out() != 1 diagnosis (Killed K7) could never
have been the site.

3. Which must land first, and does either regress the other? — guardarm
first, and no. They are separate predicates on separate inputs, so they are
two PRs. guardarm is the narrower and more obviously symmetric change (the tie
is between two folds that are both legal; new_block_if is symmetric in the arm
it takes), so it goes first and loopcondhoist stacks on it. Measured, not
assumed: the four named counter-cases are byte-identical under each option
alone and under both together, and the two options' changed-function sets
were measured separately on the same corpus.

4. What separates the ~84% already-correct panes from the inverted ones?
The tie itself, plus code layout. It must distinguish control_list's two
guards from each other, and the halt bit cannot — the trace above shows both
guards carry halt=noreturn on one arm. What differs is which arm, and the
compiler's own answer to that is the address: an unoptimised compiler emits the
taken clause of if (c) A; B; in front of B, and the then side of an
if/else in front of the else side. So: in a tie, the arm whose front leaf
lies earlier in the address space becomes the clause. This deliberately does
NOT read op_mark_halt — preferring the no-return arm fixes scp xcalloc and
control_list and inverts coreutils make-prime-list xalloc, whose source
really is if (p) return p; fprintf(...); exit(1);
(O0/coreutils/compiled/make-prime-list.i:3645). Layout gets all three right.

5. For B form C, what is the exact in-code follower assertion? — it is
already in the rule: size_in() == 1. The round required "the loop's
immediate structural follower, in-degree exactly 1" and warned that a matcher
keyed on "a return X reachable after the loop" frees libacl getfacl get_list's list and returns NULL. No new predicate is needed, because the fix
is not a hoist-and-relocate: rule_block_if_no_exit folds the body block
whose clause is the follower, and its own if size_in() != 1 { continue; }
is exactly that assertion. get_list's follower is sub_27de(v9); return NULL;
— not a bare return, reached only by the break — so it moves into the break
arm and the normal loop exit returns the list, which is the baseline's semantics
exactly. Verified on the rebased tree:

default:              while( true ) { if (v7 <= 0) return v9; ... if (!v10) break; ... }
                      sub_27de(v9); return NULL;
loopcondhoist on:     while (0 < v7) { ... if (!v10) { sub_27de(v9); return NULL; } ... }
                      return v9;

bzip2 mainGtU is dropped from the witness list as the round instructed: it
is a non-member (its source is a genuine do/while).

---

Phase 2 — what loopcondhoist does

Give the non-loop-heads one pass of the deferred ruleBlockIfNoExit scan;
fall back to the unrestricted upstream pass only when it finds nothing.
Loop-head-ness is resolved through the collapsed graph exactly the way
LoopBody::update resolves a recorded head. A function with no loop-head
candidate is byte-identical, and the scan still terminates on the same fixpoint
because every fold strictly reduces the component count.

No new transform, and no follower-move predicate — see verdict item 5: the
in-code assertion the round demanded ("the loop's immediate structural follower,
in-degree exactly 1") is rule_block_if_no_exit's own size_in() != 1 test,
which the existing rule already enforces.

function	default	loopcondhoist on
coreutils od read_char @0x5320	while( true ) { if (!dat) return v3; ... }	while (dat_12198) { ... }
libacl getfacl walk_tree_visited @0x44df	while( true ) { if (v1 == a0) return 0; ... }	for (v1 = *(a0+8); v1 != a0; v1 = *(v1+8)) { ... } return 0; — the source verbatim
probe p23 (form C)	while( true ) { if (v1 <= 0) return 0; ... break; }	for (v1 = a1; 0 < v1; v1 = v1 + -1) { if (...) return 1; } return 0; — the source; this is the new stage test
probes p7, p20	already correct at baseline	unchanged

Named counter-cases, verified on the rebased tree

Re-run on feat/loopcondhoist @a77e1ce5 (rebased onto ee14e461 / 11f40f46):

counter-case	result
bash/mksyntax main @0x19f7	byte-identical
coreutils/tail xlseek @0x4804	byte-identical
coreutils/make-prime-list xalloc @0x1906	byte-identical
libacl/getfacl get_list @0x281a	semantics preserved (below)

get_list is the named victim of a naive follower-relocation, and it is the one
counter-case that does change shape — correctly:

default:            while( true ) { if (v7 <= 0) return v9; ... if (!v10) break; ... }
                    sub_27de(v9); return NULL;

loopcondhoist on:   while (0 < v7) { ... if (!v10) { sub_27de(v9); return NULL; } ... }
                    return v9;

The free+return NULL moves into the break arm and the normal loop exit
returns the list. It does not free the list it just built. mksyntax main
is the form A counter-case (Killed K8) and is untouched because form A comes
from the is_complex(head) overflow-syntax branch, which this predicate does
not go near.

bzip2 mainGtU is dropped from the witness list per the round's own
instruction: its source is a genuine do/while, so it is a non-member.

Measurement — both directions

Full O0 slice (pre-rebase, 82dd39a7 base)

265 slices, 32,339 functions scored in both arms. Controls clean: 30,058
unchanged bodies, 0 scoring differently.

bodies changed	2,281
GED	230,291 -> 224,670 (-5,621)
GED-perfect	14,785 -> 15,454 (+669)
to perfect / off perfect	680 / 11 (61.8 : 1, McNemar z = +25.5)
improved / worsened	1,347 / 256
x86-64 / ARM Cortex-M / i386 PE	+542 / +123 / +4

This is the largest single result in the round, it beats the round's own
estimate (~210 functions / ~1,234 GED) by 3x, and no architecture loses.

Re-measured on the rebased tree (#280 funcboundflow is upstream of P8)

funcboundflow changes function boundaries, so the sweep was re-run on
a77e1ce5 over a representative 7-project slice spanning all three
architectures and containing every previously-recorded mover:
openssh-portable, betaflight, coreutils, dpkg, mydoom, libacl, zlib —
139 slices, 16,019 functions.

	pre-rebase (same 7 projects)	post-rebase
to perfect	451	451
off perfect	1	1
improved	339	341
worsened	114	115

Identical, function-for-function — the single off-perfect is
libacl/setfacl::has_execute_perms in both. Post-rebase headline for the slice:
1,284 bodies changed, GED 123,837 -> 120,233 (-3,604), perfect 6,817 -> 7,267
(+450), 451 / 1 (z = +21.17); per arch x86-64 +405, ARM +42, PE +3.

The mirror population, by name

All 11 full-corpus off-perfect functions (moved.csv); every one is x86-64,
and there are zero on ARM or PE:

bash/bash::fsleep                          0 -> 5
gnutls/certtool::read_name                 0 -> 2
gnutls/dumpcfg::read_name                  0 -> 2
gnutls/systemkey::read_name                0 -> 2
gnutls/certtool::smime_to_pkcs7            0 -> 8
gzip/gzip::inflate                         0 -> 5
libacl/setfacl::has_execute_perms          0 -> 5
libedit/libedit.so.0.0::history_list       0 -> 5
libselinux/libselinux.so::process_text_file 0 -> 6
shadow/gpasswd::change_passwd              0 -> 2
tar/tar::get_directory_entries             0 -> 5

libacl setfacl has_execute_perms — the only one in the re-measured slice — was
read against its source (O0/libacl/compiled/do_set.i:5323), which is a literal
for(;;) with two internal returns. The baseline renders it do { ... } while (...) (GED 0) and the variant renders it while( true ) { ... if (v1) break; ... } return 1;. The variant is the more faithful of the two and GED charges
5. Worth naming as an honest limit: on a genuinely-infinite source loop the pass
can move toward while( true ) rather than away from it.

Requirement 8 — every changed function, not the witness

All 2,281 changed bodies audited mechanically:

	base	var
while( true )	2,087	223
goto	621	409
label	399	251
// no-return	2,419	2,419
lines	123,744	118,891

Sixty-six functions lose duplicated callees; all were inspected and they are the
returndup-cloned loop follower being re-shared as the single loop exit.
openssh addr_match_cidr_list is the clearest — five copies of
free(v11); return v9; become one after the loop with four breaks, which is
the openssh source (for (...; (o = strsep(&cp, ",")) != NULL;) { ... goto foo; } foo: free(list); return ret;) verbatim.

Speed

Interleaved min-of-N whole-binary decompile-all on the rebased build. No
regression; plausibly a genuine speedup, since 1,864 fewer while( true )
structures means far fewer select_goto / TraceDAG rounds.

binary	base	loopcondhoist on	delta	both options on	delta
coreutils ls (min-of-5)	8,495 ms	8,469 ms	-0.31%	8,632 ms	+1.61%
openssh sshd (min-of-3, 9,173 fns)	42,486 ms	42,832 ms	+0.81%	44,447 ms	+4.62%

Both options together stay inside the 5% budget on the largest binary in the
corpus. (An earlier loaded run read ls at -26%; that was contention, not a
speedup, and does not reproduce.)

Gates — run on the REBASED tree (a77e1ce5, base ee14e461 on 11f40f46)

Exit codes captured directly (make ... > log 2>&1; echo $?), never through a
pipe to tail.

make test             exit=0   datatests: 675/675 assertions passed   ===  PARITY OK
make test-stages      exit=0   datatests: 413/413 assertions passed   ===  PARITY OK
make rust-test        exit=0   4,540 passed / 0 failed / 37 ignored
make check-spec       exit=0   check-spec OK (lenient mode)
check_spec.py --strict exit=0  check-spec OK (strict mode)
kuna catalog --check  exit=0   catalog OK: documents exactly the registered kuna options

docs/baseline-stages.json moved 407 -> 413 and the diff was inspected key by
key: +6 assertions (3 GUARDARM, 3 LOOPCONDHOIST) and the KUNA-CATALOG #7 key renamed because its assertion name embeds the option list. Nothing
removed, no existing verdict changed.

Every hard-coded catalog count re-derived from the built artifact, not by
arithmetic, and grepped back out of the file to prove the edit is not a silent
no-op. (#280 and #278 made the identical 95 -> 96 edit at four sites, git
auto-merged them with no conflict marker, and catalog_bytecompat.rs never
appeared in git status.) Read assertions, not test names — on main
settable_count_is_95 asserts 96.

counter	derived
kuna catalog --json options	98
kuna_phases/tests.rs kuna_num_settables / SETTABLE_TABLE.len	98
tier split (core, transform, analysis)	(20, 44, 34)
emit_catalog_json },\n count	97
catalog_bytecompat.rs fixture "option" / "tier" / "symptoms"	98
kuna-catalog.xml #7 structure-recovery	25
kuna-base/src/xml.rs corpus files	196
recaptured phase_catalog.json "option" entries	98

docs/options.md and tests/fixtures/phase_catalog.json regenerated from that
build; docs/baseline-stages.json re-recorded, not hand-merged. Two other
option-adding PRs are in flight (#282 P9, a P3 calloverlap) plus an or-chain
agent in p8_structure, so expect these counters to need re-deriving again
at merge time — re-derive, do not add.

Scope

Default-OFF opt-in, tier = transform. No DIV row: a new default-off
option changes no default (DIV-67 is claimed by #280, DIV-68 by #277; next free
is DIV-69, and neither of these two PRs takes one). Listed in modes.rs's
UNEVALUATED with guardarm.

Given +669 perfect with no architecture losing and an 11-function mirror
population, this is the stronger of the two candidates for the preset flip —
that PR's remaining work is the 0/675 ablation with the flag forced on and a
preset-level speed number.

Spec prose: docs/spec/08-structuring.md section 8.1.

Merge order

Intended order for this pair:

1. Retarget this PR's base from feat/guardarm to main first:
   gh api -X PATCH repos/Noelo-Lab/kuna/pulls/283 -f base=main
   (gh pr edit is broken in this environment; use the API form.)
2. Merge [AUTOMATED] feat(p8): guardarm - break the ruleBlockIfNoExit arm TIE by code layout (+297 GED-perfect / 32,339 O0 functions) #281 (guardarm) -> main.
3. Re-derive this PR's catalog counters, then merge [AUTOMATED] feat(p8): loopcondhoist - defer a loop head in the ifNoExit scan so ruleBlockWhileDo keeps the head test (+669 GED-perfect / 32,339 O0 functions) #283 -> main.

delete_branch_on_merge is set repo-wide, so merging #281 deletes
feat/guardarm and auto-closes this PR unless step 1 happens first.

Sequencing against the other option-adding PRs in flight. #278 and #284 both
land the catalog on 97 and are mutually exclusive at that value; #282 (P9) and
the P3 calloverlap add options too. Whichever of the five merges after another
needs its counters re-derived from a green build, not incremented. Concretely
for this pair: #281 asserts 97 and #283 asserts 98 against today's
main (96). If any other option-adding PR merges first, both numbers move and
must be re-derived — the four sites that merge clean at a wrong value are
catalog_bytecompat.rs (its fixture_has_all_N_settables + three
.matches(...).count() asserts), the kuna_num_settables / SETTABLE_TABLE.len
pair, emit_catalog_json_static_form_brackets_and_commas's trailing-comma tally,
and tier_counts_are_*.

🤖 Generated with Claude Code

https://claude.ai/code/session_01C8UQbPqALzdUQ3cLLjUeKH