commit 70d7ac3b3443aaf7afc32adeb979cbcfd4953444
parent 13beba6de38f1389ec3217e78f27ddacfabb57bc
Author: Ryan Sepassi <rsepassi@gmail.com>
Date: Wed, 10 Jun 2026 08:28:02 -0700
cleanup: I.4 — route addr-CSE + SSA use-rewrite through central operand walk
pass_addr_fold's addr_cse_apply_to_inst and pass_ssa's reg_replace_inst_uses
each re-hand-rolled the main-opnds + IR_CALL/RET/ASM/INTRINSIC aux traversal.
Both are uniform over the operands the central opt_walk_inst_operands already
visits (it decomposes OPK_INDIRECT to base/index), so route them through it:
addr-CSE via a per-register remap callback, SSA via a use-only callback (defs
are renamed separately). Drops the now-dead reg_replace_abivalue_uses.
reg_define_inst_defs (needs per-def index + version stack) and
opt_collect_inst_uses (needs operand index + IR_PHI preds) stay hand-rolled —
the generic (Operand*, is_def) callback can't carry that context — as does
opt_hard_inst_use_def. Documented in TODO.
No behavior change: test-opt (cg_control/cg_switch/strength_reduce/
panic_recovery/cg_fp_cmp all 0 fail), test-cg-api 211/0, test-toy 1392/0/35.
Diffstat:
3 files changed, 44 insertions(+), 124 deletions(-)
diff --git a/doc/plan/TODO.md b/doc/plan/TODO.md
@@ -91,8 +91,13 @@ Add new deferred fixes below as they are discovered.
flag-parse blocks were assessed and are not byte-identical — different option structs.)
- **I.2 — two parallel C-6.7.9 initializer walkers** (`lang/c/parse/parse_init.c:648-777`)
re-encode the same traversal grammar. Factor a single grammar driver + leaf vtable.
-- **I.4 — opt passes re-hand-roll the centralized operand-walk.** (The `ranges_overlap`
- trampoline part is fixed.)
+- **I.4 — two opt walks still hand-roll operand iteration that the central
+ `opt_walk_inst_operands` cannot express.** `reg_define_inst_defs` (pass_ssa) needs a
+ per-def index + version-stack (`pushed`) context, and `opt_collect_inst_uses`
+ (pass_analysis) needs the operand index + IR_PHI pred-value handling — neither fits
+ the generic `(Operand*, is_def)` callback without losing information. (`addr_cse_apply_to_inst`
+ and the SSA use-rewrite `reg_replace_inst_uses` now route through the central walk;
+ `opt_hard_inst_use_def` stays intentionally bespoke per its hardcoded opcode dispatch.)
## God-function decompositions (highest risk / lowest ROI — do when next touching)
diff --git a/src/opt/pass_addr_fold.c b/src/opt/pass_addr_fold.c
@@ -551,72 +551,29 @@ static u32 addr_cse_find_or_add(AddrCseEntry** entries, u32* n, u32* cap,
return idx;
}
-static void addr_cse_apply_to_operand(Operand* op, const PReg* remap) {
- /* remap is zero-initialized; 0 means "no remap" (preg 0 is reserved as
- * unused). PREG_NONE = 0xffffffff and would be a valid remap target but
- * we never produce that. */
- if (!op) return;
- if (op->kind == OPK_REG) {
- PReg p = (PReg)op->v.reg;
- if (p != PREG_NONE && p != 0 && remap[p] != 0) op->v.reg = remap[p];
- } else if (op->kind == OPK_INDIRECT) {
- PReg p = (PReg)op->v.ind.base;
- if (p != PREG_NONE && p != 0 && remap[p] != 0) op->v.ind.base = remap[p];
- if (op->v.ind.index != (Reg)REG_NONE) {
- PReg pi = (PReg)op->v.ind.index;
- if (pi != PREG_NONE && pi != 0 && remap[pi] != 0)
- op->v.ind.index = remap[pi];
- }
- }
+/* Per-register remap callback for the centralized operand walk. The walk
+ * decomposes OPK_INDIRECT into its OPK_REG base/index and writes them back, so
+ * a plain register remap here covers indirects too. `remap` is zero-init; 0
+ * means "no remap" (preg 0 is reserved unused; we never produce PREG_NONE as a
+ * target). */
+static void addr_cse_remap_op(Func* f, Inst* in, Operand* op, int is_def,
+ void* ctx) {
+ const PReg* remap = (const PReg*)ctx;
+ PReg p;
+ (void)f;
+ (void)in;
+ (void)is_def;
+ if (!op || op->kind != OPK_REG) return;
+ p = (PReg)op->v.reg;
+ if (p != PREG_NONE && p != 0 && remap[p] != 0) op->v.reg = remap[p];
}
-static void addr_cse_apply_to_inst(Inst* in, const PReg* remap) {
- for (u32 o = 0; o < in->nopnds; ++o)
- addr_cse_apply_to_operand(&in->opnds[o], remap);
- /* IR_CALL aux carries operands too; rewrite both replay variants. */
- if ((IROp)in->op == IR_CALL) {
- IRCallAux* aux = (IRCallAux*)in->extra.aux;
- if (!aux) return;
- if (aux->use_plan_replay) {
- addr_cse_apply_to_operand(&aux->plan.callee, remap);
- for (u32 i = 0; i < aux->plan.nargs; ++i)
- addr_cse_apply_to_operand(&aux->plan.args[i].src, remap);
- for (u32 i = 0; i < aux->plan.nrets; ++i)
- addr_cse_apply_to_operand(&aux->plan.rets[i].dst, remap);
- } else {
- addr_cse_apply_to_operand(&aux->desc.callee, remap);
- for (u32 i = 0; i < aux->desc.nargs; ++i) {
- CGABIValue* v = (CGABIValue*)&aux->desc.args[i];
- addr_cse_apply_to_operand(&v->storage, remap);
- for (u32 k = 0; k < v->nparts; ++k)
- addr_cse_apply_to_operand((Operand*)&v->parts[k].op, remap);
- }
- addr_cse_apply_to_operand(&aux->desc.ret.storage, remap);
- for (u32 k = 0; k < aux->desc.ret.nparts; ++k)
- addr_cse_apply_to_operand((Operand*)&aux->desc.ret.parts[k].op, remap);
- }
- } else if ((IROp)in->op == IR_RET) {
- IRRetAux* aux = (IRRetAux*)in->extra.aux;
- if (aux && aux->present) {
- addr_cse_apply_to_operand(&aux->val.storage, remap);
- for (u32 k = 0; k < aux->val.nparts; ++k)
- addr_cse_apply_to_operand((Operand*)&aux->val.parts[k].op, remap);
- }
- } else if ((IROp)in->op == IR_ASM_BLOCK) {
- IRAsmAux* aux = (IRAsmAux*)in->extra.aux;
- if (!aux) return;
- for (u32 i = 0; i < aux->nin; ++i)
- addr_cse_apply_to_operand(&aux->in_ops[i], remap);
- for (u32 i = 0; i < aux->nout; ++i)
- addr_cse_apply_to_operand(&aux->out_ops[i], remap);
- } else if ((IROp)in->op == IR_INTRINSIC) {
- IRIntrinAux* aux = (IRIntrinAux*)in->extra.aux;
- if (!aux) return;
- for (u32 i = 0; i < aux->narg; ++i)
- addr_cse_apply_to_operand(&aux->args[i], remap);
- for (u32 i = 0; i < aux->ndst; ++i)
- addr_cse_apply_to_operand(&aux->dsts[i], remap);
- }
+static void addr_cse_apply_to_inst(Func* f, Inst* in, const PReg* remap) {
+ /* The centralized walk visits every operand the bespoke walk did — main
+ * opnds plus IR_CALL/RET/ASM/INTRINSIC aux — so a uniform per-register remap
+ * is equivalent. (It also visits an IR_PARAM_DECL's def, which is never a
+ * CSE target, so remap leaves it untouched.) */
+ opt_walk_inst_operands(f, in, addr_cse_remap_op, (void*)remap);
}
Inst* opt_block_insert_at(Func* f, Block* bl, u32 at, u32 k) {
@@ -742,7 +699,7 @@ void opt_addr_of_global_cse(Func* f) {
for (u32 b = 0; b < f->nblocks; ++b) {
Block* bl = &f->blocks[b];
for (u32 i = 0; i < bl->ninsts; ++i) {
- addr_cse_apply_to_inst(&bl->insts[i], remap);
+ addr_cse_apply_to_inst(f, &bl->insts[i], remap);
}
}
@@ -918,7 +875,7 @@ void opt_hoist_loop_consts(Func* f) {
for (u32 b = 0; b < f->nblocks; ++b) {
Block* bl = &f->blocks[b];
for (u32 i = 0; i < bl->ninsts; ++i)
- addr_cse_apply_to_inst(&bl->insts[i], remap);
+ addr_cse_apply_to_inst(f, &bl->insts[i], remap);
}
opt_analysis_invalidate(
diff --git a/src/opt/pass_ssa.c b/src/opt/pass_ssa.c
@@ -303,13 +303,6 @@ static void reg_replace_use(RegRenameCtx* ctx, Operand* op) {
}
}
-static void reg_replace_abivalue_uses(RegRenameCtx* ctx, CGABIValue* v) {
- if (!v) return;
- reg_replace_use(ctx, &v->storage);
- for (u32 i = 0; i < v->nparts; ++i)
- reg_replace_use(ctx, (Operand*)&v->parts[i].op);
-}
-
static Val reg_define_operand(RegRenameCtx* ctx, u32 b, u32 i, Inst* in,
Operand* op, u32 def_index, u32* pushed) {
if (!op || op->kind != OPK_REG) return VAL_NONE;
@@ -342,56 +335,21 @@ static u32 reg_def_index(const Inst* in, Reg r, u32 ordinal) {
return 0;
}
-static void reg_replace_inst_uses(RegRenameCtx* ctx, Inst* in) {
- for (u32 o = 0; o < in->nopnds; ++o) {
- Operand* op = &in->opnds[o];
- int is_def = 0;
- if (op->kind == OPK_REG) {
- if ((IROp)in->op == IR_ATOMIC_CAS)
- is_def = (o == 0 || o == 1) && reg_in_defs(in, op->v.reg);
- else
- is_def = o == 0 && reg_in_defs(in, op->v.reg);
- }
- if (!is_def) reg_replace_use(ctx, op);
- }
+static void reg_replace_use_cb(Func* f, Inst* in, Operand* op, int is_def,
+ void* ctx) {
+ (void)f;
+ (void)in;
+ if (!is_def) reg_replace_use((RegRenameCtx*)ctx, op);
+}
- switch ((IROp)in->op) {
- case IR_CALL: {
- IRCallAux* aux = (IRCallAux*)in->extra.aux;
- if (!aux) break;
- if (aux->use_plan_replay) {
- reg_replace_use(ctx, &aux->plan.callee);
- for (u32 i = 0; i < aux->plan.nargs; ++i)
- reg_replace_use(ctx, &aux->plan.args[i].src);
- } else {
- reg_replace_use(ctx, &aux->desc.callee);
- for (u32 i = 0; i < aux->desc.nargs; ++i)
- reg_replace_abivalue_uses(ctx, (CGABIValue*)&aux->desc.args[i]);
- }
- break;
- }
- case IR_RET: {
- IRRetAux* aux = (IRRetAux*)in->extra.aux;
- if (aux && aux->present) reg_replace_abivalue_uses(ctx, &aux->val);
- break;
- }
- case IR_SCOPE_BEGIN:
- break;
- case IR_ASM_BLOCK: {
- IRAsmAux* aux = (IRAsmAux*)in->extra.aux;
- if (!aux) break;
- for (u32 i = 0; i < aux->nin; ++i) reg_replace_use(ctx, &aux->in_ops[i]);
- break;
- }
- case IR_INTRINSIC: {
- IRIntrinAux* aux = (IRIntrinAux*)in->extra.aux;
- if (!aux) break;
- for (u32 i = 0; i < aux->narg; ++i) reg_replace_use(ctx, &aux->args[i]);
- break;
- }
- default:
- break;
- }
+static void reg_replace_inst_uses(RegRenameCtx* ctx, Inst* in) {
+ /* Route through the centralized operand walk: it applies the same use/def
+ * classification (incl. the IR_ATOMIC_CAS two-def case) and visits the same
+ * IR_CALL/RET/ASM/INTRINSIC aux operands, decomposing OPK_INDIRECT into its
+ * base/index — exactly what the bespoke walk did. The callback rewrites only
+ * uses; defs are renamed separately (reg_define_inst_defs, which needs the
+ * per-def index/version-stack context the generic callback can't carry). */
+ opt_walk_inst_operands(ctx->f, in, reg_replace_use_cb, ctx);
}
static void reg_define_abivalue(RegRenameCtx* ctx, u32 b, u32 i, Inst* in,
