Recover branching signed /2^k division and wider Thumb jump tables

apps/web/src/data/summary.json

@@ -1,10 +1,10 @@
 {
   "total": 743,
   "match": {
-    "asmlift": 361,
+    "asmlift": 363,
     "m2c": 342
   },
-  "commit": "92b4d4019ffc6e30c7f5accaa0740669631bdf74",
+  "commit": "5d2f723fc7820cb3b3bd03539466811a529812ba",
   "m2cCommit": "ad5c529a65ca1e5191b00a4a7b4cbfe79a7b45a0",
   "dirty": false
 }

docs/asmlift-101.md

@@ -233,7 +233,8 @@ enormously for retro consoles:
   helper like `__divsi3`. Recognizers rewrite those calls back to operators
   ([`raise/softdiv.ts`](../packages/core/src/raise/softdiv.ts)).
 
-Simpler idioms (power-of-two division via shifts — the `half` example in Part III — multiply
+Simpler idioms (the branchless power-of-two division in the `half` example in Part III — its
+BRANCHING sibling is a CFG diamond and lives in a raise pass instead — multiply
 strength-reduction, width casts) are expressed as **rewrite patterns as data**
 ([`pattern/engine.ts`](../packages/core/src/pattern/engine.ts)): serializable objects saying
 "this DAG (directed-acyclic-graph) shape of operations becomes this op", most of them gated to

packages/core/src/frontend/ssa.ts

@@ -3,6 +3,14 @@
 // CFG (predecessors per block) and, per block, emits ops through `readVar`/`writeVar`; this
 // module materialises block-argument phis at joins and back-edges.
 //
+// `preds` is an EDGE list, not a block list: it carries one entry per CFG edge, so a `switch_br`
+// with several case values reaching one block appears there several times. Both readings are
+// needed and they are not interchangeable — phi wiring wants the distinct predecessor BLOCKS (one
+// value each), while the args it appends belong to the EDGES (every one of them). `distinctPreds`
+// names the first; `appendSuccessorArg` walks the second. (ir/core.ts `predecessors` and
+// structure.ts `predecessorBlocks` have the same duality, and structure.ts already dedups ad hoc
+// at its two join sites.)
+//
 // Protocol: create the builder, then fill blocks in index order. For each block, emit its
 // computation via read/writeVar, push its terminator op last (successors referencing
 // `irBlocks`, args left empty — phi wiring appends them), then call `markFilled(b)`. When all
@@ -28,6 +36,7 @@ export interface SsaBuilder {
   finish(): void;
 }
 
+/** `preds` is per-EDGE (see the module header): one entry per CFG edge into each block. */
 export function makeSsaBuilder(name: string, blockCount: number, preds: number[][]): SsaBuilder {
   const irBlocks: Block[] = Array.from({ length: blockCount }, () => ({ params: [] as Value[], ops: [] }));
   const fn: Fn = { name, blocks: irBlocks };
@@ -39,6 +48,9 @@ export function makeSsaBuilder(name: string, blockCount: number, preds: number[]
   const phiBlock = new Map<Value, number>();
   const paramReg = new Map<Value, string>();
 
+  // `preds` lists an entry per CFG EDGE; these are the distinct predecessor BLOCKS.
+  const distinctPreds = (b: number): number[] => [...new Set(preds[b])];
+
   const writeVar = (reg: string, b: number, v: Value) => defs[b].set(reg, v);
   const readVar = (reg: string, b: number): Value => defs[b].get(reg) ?? readRecursive(reg, b);
 
@@ -56,7 +68,10 @@ export function makeSsaBuilder(name: string, blockCount: number, preds: number[]
       incompletePhis[b].set(reg, phi);
       return phi;
     }
-    const ps = preds[b];
+    // DISTINCT predecessor blocks: a switch_br reaching this block on several case values is one
+    // predecessor with several edges, and it supplies ONE value — counting the edges instead would
+    // manufacture a join (and a phi) where there is none.
+    const ps = distinctPreds(b);
     if (ps.length === 0) {
       // live-in with no predecessor: an incoming argument register → function parameter.
       const p = mkValue(T.unk(32));
@@ -76,16 +91,24 @@ export function makeSsaBuilder(name: string, blockCount: number, preds: number[]
     return phi;
   };
   const addPhiOperands = (reg: string, b: number) => {
-    for (const p of preds[b]) {
+    for (const p of distinctPreds(b)) {
       appendSuccessorArg(p, b, readVar(reg, p));
     }
   };
-  // Append `arg` to predecessor p's terminator successor that targets block b.
+  // Append `arg` to EVERY successor edge of predecessor p that targets block b.
+  //
+  // A predecessor normally has one edge to a given successor, but a `switch_br` has as many as it
+  // has case values, and two cases sharing a body (`case 1: case 2:`) is ordinary C. Block args
+  // belong to the EDGE, so each of those edges needs its own copy: appending to just the first (a
+  // `find`) left the others short, while `preds` listing the block once per edge made the loop run
+  // k times and pile k copies onto that same first edge. Both halves of that — every edge, once per
+  // predecessor BLOCK — have to hold together, which is why they are fixed in one place.
   const appendSuccessorArg = (p: number, b: number, arg: Value) => {
     const term = irBlocks[p].ops[irBlocks[p].ops.length - 1];
-    const s = term.successors.find((su) => su.block === irBlocks[b]);
-    if (s) {
-      s.args.push(arg);
+    for (const s of term.successors) {
+      if (s.block === irBlocks[b]) {
+        s.args.push(arg);
+      }
     }
   };
   const sealBlock = (b: number) => {

packages/core/src/frontend/thumb.ts

@@ -650,6 +650,41 @@ function decode(name: string, asm: string): { blocks: AsmBlock[]; dataWords: Map
         `cannot lift '${name}': block '${mixed.label}' interleaves raw data (.${subwordData.get(mixed.label)}) with instructions`,
       );
     }
+    // Two labels on the same instruction (`.LCB80:` immediately followed by `.L7:`) make the first
+    // an ALIAS of the second, not a block of its own — agbcc emits exactly that when a long-jump
+    // helper label lands on an existing one. The empty block is dropped just below, so a branch
+    // naming the alias would afterwards resolve to nothing and decline as a dangling target. Point
+    // those branches at the block the label actually names, before anything reads the CFG.
+    // A label naming DATA is emphatically NOT an alias, and this is the guard the whole pass turns
+    // on. Decode pushes an empty block for a literal-pool / jump-table label too, so aliasing them
+    // blindly would silently retarget `beq .Lpool` at whatever code happens to follow the pool —
+    // marker-free, plausible, wrong C where the frontend used to decline. Every agbcc pool is a
+    // label on data, so that is the common case, not an exotic one. A data label therefore neither
+    // aliases nor is aliased THROUGH: scanning past one for a later code block would silently jump
+    // over the data.
+    const isDataLabel = (l: string) => dataWords.has(l) || subwordData.has(l);
+    const aliasOf = new Map<string, string>();
+    for (let i = 0; i < blocks.length; i++) {
+      if (blocks[i].instrs.length > 0 || isDataLabel(blocks[i].label)) {
+        continue;
+      }
+      let j = i + 1;
+      while (j < blocks.length && blocks[j].instrs.length === 0 && !isDataLabel(blocks[j].label)) {
+        j++;
+      }
+      const next = blocks[j];
+      if (next && next.instrs.length > 0) {
+        aliasOf.set(blocks[i].label, next.label);
+      } // otherwise a trailing or data-fronted label: left dangling so a branch to it still declines
+    }
+    for (const b of aliasOf.size ? blocks : []) {
+      for (const ins of b.instrs) {
+        const k = ins.ops.length - 1;
+        if ((ins.mnemonic === 'b' || COND_OPCODE[ins.mnemonic]) && k >= 0) {
+          ins.ops[k] = aliasOf.get(ins.ops[k]) ?? ins.ops[k];
+        }
+      }
+    }
     let live = blocks.filter((b) => b.instrs.length > 0);
     // Alignment-pad NOPs a splitter emits around returns and literal pools: `lsls r0, r0, #0`
     // is the 0x0000 halfword, `mov r8, r8` is 0x46C0, plus a literal `nop`. A block made ONLY
@@ -826,21 +861,47 @@ function recoverJumpTable(
   disp: AsmBlock,
   dataWords: Map<string, string[]>,
   blockLabels: Set<string>,
+  longDefault?: string,
 ): JumpTable | null {
-  // bounds: last two instrs must be `cmp rX,#M` then `bhi DEF` (unsigned upper-bound guard).
+  // bounds: last two instrs are `cmp rX,#M` then the out-of-range guard, in one of two spellings.
+  //
+  //   direct    cmp rX,#M ; bhi DEF                 → fall through to the dispatch
+  //   long jump cmp rX,#M ; bls DISP ; b DEF        → branch TO the dispatch, long-branch the default
+  //
+  // The second is what agbcc emits whenever the default is out of a conditional branch's reach —
+  // Thumb-1 `B<cond>` carries a signed 8-bit HALFWORD offset, so ±256 BYTES, about 128
+  // instructions — which on a real switch it usually is: five of the six benchmark
+  // functions with a table use it, and only the sixth uses the direct form. `longDefault` is the
+  // target of that trailing `b`, read by the caller from the block after `bounds`.
   const bi = bounds.instrs;
-  const bhi = bi[bi.length - 1],
+  const guard = bi[bi.length - 1],
     cmp = bi[bi.length - 2];
-  if (!bhi || !cmp || bhi.mnemonic !== 'bhi' || cmp.mnemonic !== 'cmp') {
+  if (!guard || !cmp || cmp.mnemonic !== 'cmp') {
     return null;
   }
+  let defaultLabel: string;
+  if (longDefault === undefined) {
+    if (guard.mnemonic !== 'bhi') {
+      return null;
+    }
+    defaultLabel = guard.ops[0];
+  } else {
+    // The `bls` must name THIS dispatch block, or the guard belongs to some other branch and the
+    // `b` we picked up is not its default.
+    if (guard.mnemonic !== 'bls' || guard.ops[0] !== disp.label) {
+      return null;
+    }
+    defaultLabel = longDefault;
+  }
   const scrutReg = cmp.ops[0];
   const m = cmp.ops[1];
   if (!m?.startsWith('#')) {
     return null;
   }
   const n = imm(m) + 1; // cases 0..M  → N = M+1
-  const defaultLabel = bhi.ops[0];
+  if (n < 1) {
+    return null; // a bound that admits no case at all is not a dispatch — fail closed
+  }
 
   // disp: exactly the 5-op idiom, threading a single index register from `lsl rY,rX,#2`.
   const d = disp.instrs;
@@ -877,11 +938,28 @@ function recoverJumpTable(
   }
 
   // Read the table: the ldr loads a POINTER word (PTR: .word TABLE); the table is TABLE: .word C0…
-  const ptrWords = dataWords.get(ptrLabel);
-  if (!ptrWords || ptrWords.length !== 1) {
+  // Note the case labels are matched against `blockLabels` as WRITTEN: the adjacent-label aliasing in
+  // `decode` rewrites branch operands, not `.word` entries, so a table naming an aliased label would
+  // decline here rather than dispatch anywhere. Loud, and no corpus instance — left as a known edge
+  // rather than fixed speculatively.
+  //
+  // The pointer word is addressed the same way every other pool load in this frontend is —
+  // `LABEL[+N]`, selecting word N/4 — because a literal pool is a POOL: agbcc packs the dispatch
+  // pointer in beside whatever else the function needed, and which slot it lands in is an artifact
+  // of emission order. Reading only a bare label whose pool held exactly ONE word declined six real
+  // benchmark functions whose table pointer merely sat later in the pool. Same fix m2c made in
+  // `a7c5c2d`, and the same shared POOL_LABEL grammar the const/gaddr resolvers use, so the three
+  // cannot disagree about what `.L21+0x4` addresses.
+  const pm = ptrLabel.match(POOL_LABEL);
+  const ptrWords = pm ? dataWords.get(pm[1]) : undefined;
+  if (!pm || !ptrWords) {
     return null;
   }
-  const caseLabels = dataWords.get(ptrWords[0]);
+  const ptrOff = pm[2] ? Number(pm[2]) : 0;
+  if (ptrOff % 4 !== 0 || ptrOff / 4 >= ptrWords.length) {
+    return null; // misaligned or past the end of the pool — not a word this pool holds
+  }
+  const caseLabels = dataWords.get(ptrWords[ptrOff / 4].trim());
   if (!caseLabels || caseLabels.length !== n) {
     return null;
   } // table length must equal the bound
@@ -917,26 +995,49 @@ export function lift(
   const poolNamesSymbols = poolNamesASymbol(dataWords, blockLabels);
   // Any label referenced as a branch target (so we can tell if an elided dispatch block has a SECOND
   // predecessor — a `b disp` from elsewhere — which would dangle after elision; decline if so).
-  const branchTargets = new Set<string>();
+  // How many branches name each label — not just whether any does, because the long-jump bounds
+  // form legitimately branches to its own dispatch block exactly once.
+  const branchRefs = new Map<string, number>();
   for (const b of rawBlocks) {
     for (const ins of b.instrs) {
       if ((ins.mnemonic === 'b' || COND_OPCODE[ins.mnemonic]) && ins.ops.length) {
-        branchTargets.add(ins.ops[ins.ops.length - 1]);
+        const t = ins.ops[ins.ops.length - 1];
+        branchRefs.set(t, (branchRefs.get(t) ?? 0) + 1);
       }
     }
   }
   const tables = new Map<AsmBlock, JumpTable>(); // bounds block → recovered table
-  const elided = new Set<AsmBlock>(); // dispatch blocks removed from the CFG
+  const elided = new Set<AsmBlock>(); // dispatch (and long-jump default) blocks removed from the CFG
   rawBlocks.forEach((d, i) => {
     const last = d.instrs[d.instrs.length - 1];
-    if (last && last.mnemonic === 'mov' && last.ops[0] === 'pc' && last.ops[1] !== 'lr') {
-      const bounds = rawBlocks[i - 1];
-      // The dispatch block must be reached ONLY by falling through from its bounds predecessor — a
-      // `b disp` target elsewhere would leave a dangling edge after elision, so decline (→ loud-fail).
-      const jt = bounds && !branchTargets.has(d.label) ? recoverJumpTable(bounds, d, dataWords, blockLabels) : null;
+    if (!last || last.mnemonic !== 'mov' || last.ops[0] !== 'pc' || last.ops[1] === 'lr') {
+      return;
+    }
+    const refs = branchRefs.get(d.label) ?? 0;
+    const prev = rawBlocks[i - 1];
+    // Direct form: the dispatch is reached ONLY by falling through from its bounds predecessor. A
+    // `b disp` from anywhere else would leave a dangling edge after elision, so decline (→ loud-fail).
+    if (prev && refs === 0) {
+      const jt = recoverJumpTable(prev, d, dataWords, blockLabels);
+      if (jt) {
+        tables.set(prev, jt);
+        elided.add(d);
+        return;
+      }
+    }
+    // Long-jump form: `bounds` (cmp; bls DISP), then a lone `b DEF` block, then the dispatch. The
+    // dispatch is entered by exactly that one `bls` and nothing else, and the `b DEF` block —
+    // synthetically labelled, so unnameable and unreachable once the bounds block dispatches — is
+    // elided WITH it. Leaving it would make it a parameterless predecessor of the default block,
+    // and wiring a phi through it fabricates an entry parameter (the phantom-param miscompile).
+    const boundsB = rawBlocks[i - 2];
+    const prevNamed = prev ? (branchRefs.get(prev.label) ?? 0) > 0 : false;
+    if (refs === 1 && prev && boundsB && !prevNamed && prev.instrs.length === 1 && prev.instrs[0].mnemonic === 'b') {
+      const jt = recoverJumpTable(boundsB, d, dataWords, blockLabels, prev.instrs[0].ops[0]);
       if (jt) {
-        tables.set(bounds, jt);
+        tables.set(boundsB, jt);
         elided.add(d);
+        elided.add(prev);
       }
     }
   });