Section 05: Structural Derive Normalization
Status: Not Started
Goal: Derived trait methods (Eq, Comparable, Hashable) have known properties: they only read their arguments, never allocate, and have deterministic results. Marking them memory(read) lets LLVM’s GVN eliminate redundant calls (e.g., calling x.eq(y) twice in the same basic block without intervening writes). The reflexive and boolean peepholes remain documented here because they motivated the section, but their executable implementation anchor is Section 09’s shared ARC normalization pass.
Success Criteria:
-
ORI_DUMP_AFTER_LLVM=1showsmemory(read)on derived Eq/Comparable/Hashable method functions - LLVM GVN eliminates second call when
point.eq(other)is called twice without intervening store - Section 09 is the sole implementation anchor for reflexive/boolean peepholes; Section 05 requires no duplicate ARC peephole pass
Context: The DerivedTrait system already has is_nounwind_derived() (derives/mod.rs:218) which marks derived Eq/Comparable/Clone/Hashable/Default as nounwind. The same pattern applies for memory(read) — these methods read fields but never write or allocate. For reflexive peepholes: derived structural Eq always returns true when both operands are the same variable (because field-by-field comparison of identical values is trivially true). Float Eq is excluded because NaN != NaN.
Depends on: Nothing for 05.1 (memory(read) attributes). Subsections 05.2 and 05.3 operate at the ARC IR level and their peepholes overlap with Section 09’s normalization pass. Implementation order: 05.1 in Phase 1, 05.2/05.3 deferred to Phase 4 and implemented AS PART OF Section 09 (identity elimination subsumes boolean identity; involutive rewrite subsumes double negation; reflexive peephole shares the same ARC IR walker). This avoids building redundant peephole infrastructure that Section 09 immediately replaces. The items remain documented in Section 05 for completeness but the implementation anchor is in Section 09.
Implementation note (05.2/05.3): These peepholes operate on ARC IR (ArcValue::PrimOp), NOT LLVM codegen (ValueId). See subsection details for why.
05.1 memory(read) on Derived Methods
File(s): compiler/ori_ir/src/derives/mod.rs, compiler/ori_llvm/src/codegen/derive_codegen/mod.rs
Follow the is_nounwind_derived() pattern: add is_readonly_derived() metadata predicate to DerivedTrait, then apply the attribute at function declaration time in derive codegen.
-
Add
is_readonly_derived(&self) -> boolmethod toDerivedTraitinderives/mod.rs:- Returns
truefor: Eq, Comparable, Hashable (read fields only, never allocate) - Returns
falsefor: Clone (allocates), Default (allocates), Printable (allocates strings), Debug (allocates strings)
- Returns
-
Write failing AOT test FIRST (TDD): call
point.eq(other)twice in sequence → before the fix, IR shows two calls. This test will verify GVN elimination after the fix. -
In
derive_codegen/mod.rssetup_derive_function()(at line 272 whereis_nounwind_derivedis checked): addif trait_kind.is_readonly_derived() { fc.builder_mut().add_memory_read_attribute(func_id); } -
Verify
add_memory_read_attributeexists on IrBuilder (or add it inattributes.rs— currentlyattributes.rshas function-level attribute methods at 287 lines; check if amemory(read)attribute method already exists) -
Verify AOT test now passes: IR shows single call (GVN eliminated the duplicate)
-
Verify:
timeout 150 cargo t -p ori_llvmpasses (debug AND release) -
/tpr-reviewpassed — independent review found no critical or major issues (or all findings triaged) -
/impl-hygiene-reviewpassed — hygiene review clean. MUST run AFTER/tpr-reviewis clean. -
Subsection close-out (05.1) — MANDATORY before starting the next subsection. Run
/improve-toolingretrospectively on THIS subsection’s debugging journey (per.claude/skills/improve-tooling/SKILL.md“Per-Subsection Workflow”): whichdiagnostics/scripts you ran, where you addeddbg!/tracingcalls, where output was hard to interpret, where test failures gave unhelpful messages, where you ran the same command sequence repeatedly. Forward-look: what tool/log/diagnostic would shorten the next regression in this code path by 10 minutes? Implement improvements NOW (zero deferral) and commit each via SEPARATE/commit-pushusing a valid conventional-commit type (build(diagnostics): ... — surfaced by section-05.1 retrospective—build/test/chore/ci/docsare valid;tools(...)is rejected by the lefthook commit-msg hook). Mandatory even when nothing felt painful. If genuinely no gaps, document briefly: “Retrospective 05.1: no tooling gaps”. Update this subsection’sstatusin section frontmatter tocomplete. -
/sync-claudesection-close doc sync — verify Claude artifacts across all section commits. Map changed crates to rules files, check CLAUDE.md, canon.md. Fix drift NOW. -
Repo hygiene check — run
diagnostics/repo-hygiene.sh --checkand clean any detected temp files.
05.2 Reflexive Peepholes
File(s): compiler/ori_arc/src/aims/normalize/ (NEW, ARC IR level), NOT ori_llvm codegen
IMPORTANT: This peephole CANNOT be implemented at the LLVM codegen level (emit_binary_op()). The codegen receives ValueId (opaque LLVM values), NOT ArcVarId (original variable identity). By the time emit_binary_op() runs, the original variable identity that would let us detect x == x (same source variable) is lost — two ValueIds could be the same underlying variable but the codegen has no way to know.
The reflexive peephole must operate on ARC IR, where ArcValue::PrimOp { op: PrimOp::Binary(Eq), args: [v1, v2] } still carries ArcVarId values. When v1 == v2 (same ArcVarId), we can safely replace with a literal.
Derived-vs-custom provenance: At ARC IR level, ALL == / != operators (both primitive and user-defined types) lower to PrimOp::Binary(Eq) / PrimOp::Binary(NotEq). There is no Apply path for operator syntax. The type checker validates Eq trait satisfaction but the AST retains ExprKind::Binary through canonicalization and lowering.
For the reflexive peephole (x == x → true), this means:
- Primitive types (int, bool, char, byte, str): reflexive is trivially correct (non-float). Safe.
- User types with derived Eq: structural field-by-field comparison of identical values is trivially true. Safe.
- User types with custom Eq: custom
equalsmethod could do ANYTHING (side effects, non-reflexive semantics). The reflexive peephole would be UNSOUND. - Float:
NaN == NaNis false. UNSOUND.
The safe approach for this plan is narrower: only fire for builtin non-float primitive comparisons (int, bool, char, byte, str). User structs and custom/derived Eq remain excluded unless a future plan exports explicit Eq provenance into ARC IR. This is conservative but sound, and matches Section 09’s execution scope.
- In the ARC IR normalization pass (or a dedicated peephole pass at step 3b): scan for
ArcInstr::Let { value: ArcValue::PrimOp { op: PrimOp::Binary(Eq), args: [v1, v2] }, .. }wherev1 == v2 - If same ArcVarId AND the type is a builtin non-float primitive → replace value with
ArcValue::Literal(LitValue::Bool(true)) - Similarly for
PrimOp::Binary(NotEq)with same ArcVarId →LitValue::Bool(false) - For comparison operators with same operand:
x < x→false,x <= x→true,x > x→false,x >= x→true- Note:
compare(x, x)goes throughApply(trait method call), notPrimOp— skip for now
- Soundness guard: ONLY for builtin non-float primitives. Float NaN makes
NaN == NaN→ false, and user-defined/custom Eq is not proven reflexive byPrimOpalone. - Write Ori spec test (include
use std.testing { assert_eq }):assert_eq(actual: (x == x), expected: true)for int → verify it compiles to constant - Write negative test: float
x == xwherex = 0.0 / 0.0(NaN) → must remain as PrimOp, not constant true
Matrix dimensions:
-
Types:
int,bool,char,byte,str(safe — builtin non-float primitives),float(excluded — NaN), user struct (excluded — custom Eq possible; future work) -
Patterns:
x == x,x != x,x < x,x <= x,x > x,x >= x -
Semantic pin: int
x == xproducesLitValue::Bool(true)in ARC IR (only passes with peephole) -
Negative pin: float NaN
x == xis NOT optimized to true -
/tpr-reviewpassed — independent review found no critical or major issues (or all findings triaged) -
/impl-hygiene-reviewpassed — hygiene review clean. MUST run AFTER/tpr-reviewis clean. -
Subsection close-out (05.2) — MANDATORY before starting the next subsection. Run
/improve-toolingretrospectively on THIS subsection’s debugging journey (per.claude/skills/improve-tooling/SKILL.md“Per-Subsection Workflow”): whichdiagnostics/scripts you ran, where you addeddbg!/tracingcalls, where output was hard to interpret, where test failures gave unhelpful messages, where you ran the same command sequence repeatedly. Forward-look: what tool/log/diagnostic would shorten the next regression in this code path by 10 minutes? Implement improvements NOW (zero deferral) and commit each via SEPARATE/commit-pushusing a valid conventional-commit type (build(diagnostics): ... — surfaced by section-05.2 retrospective—build/test/chore/ci/docsare valid;tools(...)is rejected by the lefthook commit-msg hook). Mandatory even when nothing felt painful. If genuinely no gaps, document briefly: “Retrospective 05.2: no tooling gaps”. Update this subsection’sstatusin section frontmatter tocomplete. -
/sync-claudesection-close doc sync — verify Claude artifacts across all section commits. Map changed crates to rules files, check CLAUDE.md, canon.md. Fix drift NOW. -
Repo hygiene check — run
diagnostics/repo-hygiene.sh --checkand clean any detected temp files.
05.3 Boolean and Bitwise Simplifications
File(s): compiler/ori_arc/src/aims/normalize/ (ARC IR level peepholes)
Simple boolean/bitwise algebra rewrites that fire at ARC IR level (step 3b).
CRITICAL IR FACT: && / || are NEVER PrimOps. The lower_binary() function in compiler/ori_arc/src/lower/expr/mod.rs intercepts BinaryOp::And and BinaryOp::Or BEFORE they reach PrimOp emission and routes them to lower_short_circuit_and() / lower_short_circuit_or(), producing control-flow IR (branches). Only bitwise &/| (BitAnd/BitOr) reach PrimOp. Therefore:
-
x && true → x/x && false → false/x || false → x/x || true → truetarget non-existent IR and are removed from this section. -
!!x → x(UnaryOp::Notvia PrimOp) IS valid and remains. -
BitAnd/BitOr identity patterns (
x & 0xFF...FF → x,x | 0 → x) are valid PrimOp patterns and are added. -
!!x → x: scan forArcValue::PrimOp { op: PrimOp::Unary(Not), args: [y] }whereyis defined byArcValue::PrimOp { op: PrimOp::Unary(Not), args: [x] }→ replace withArcValue::Var(x) -
x & 0xFF...FF → x(BitAnd with all-ones identity): scan forPrimOp::Binary(BitAnd)where one arg isLitValue::Int(-1)(all bits set) → replace withArcValue::Var(other_arg) -
x | 0 → x(BitOr with zero identity): scan forPrimOp::Binary(BitOr)where one arg isLitValue::Int(0)→ replace withArcValue::Var(other_arg) -
x ^ 0 → x(BitXor with zero identity): scan forPrimOp::Binary(BitXor)where one arg isLitValue::Int(0)→ replace withArcValue::Var(other_arg) -
x & 0 → 0(BitAnd annihilator): scan forPrimOp::Binary(BitAnd)where one arg isLitValue::Int(0)→ replace withArcValue::Literal(LitValue::Int(0)) -
x | 0xFF...FF → 0xFF...FF(BitOr annihilator): scan forPrimOp::Binary(BitOr)where one arg isLitValue::Int(-1)→ replace withArcValue::Literal(LitValue::Int(-1)) -
Write AOT tests for each simplification pattern
-
Interaction with Section 09: These peepholes are a subset of the algebraic normalization pass.
!!x → xis the involutive rewrite for Not. BitAnd/BitOr/BitXor identity elimination falls under Section 09’s identity elimination. If Section 09 is done first, this subsection is a no-op. If this section is done first, the peepholes should be placed in a location that Section 09 can reuse.
Matrix dimensions:
-
Types:
bool(Not),int(BitAnd, BitOr, BitXor) -
Patterns:
!!x,x & -1,x | 0,x ^ 0,x & 0,x | -1,!true,!false -
Semantic pin:
!!flagcompiles toflag(no Not instructions in IR) -
Negative pin:
x && truedoes NOT appear as PrimOp (it’s control-flow IR from short-circuit lowering — no ARC IR peephole possible) -
/tpr-reviewpassed — independent review found no critical or major issues (or all findings triaged) -
/impl-hygiene-reviewpassed — hygiene review clean. MUST run AFTER/tpr-reviewis clean. -
Subsection close-out (05.3) — MANDATORY before starting the next subsection. Run
/improve-toolingretrospectively on THIS subsection’s debugging journey (per.claude/skills/improve-tooling/SKILL.md“Per-Subsection Workflow”): whichdiagnostics/scripts you ran, where you addeddbg!/tracingcalls, where output was hard to interpret, where test failures gave unhelpful messages, where you ran the same command sequence repeatedly. Forward-look: what tool/log/diagnostic would shorten the next regression in this code path by 10 minutes? Implement improvements NOW (zero deferral) and commit each via SEPARATE/commit-pushusing a valid conventional-commit type (build(diagnostics): ... — surfaced by section-05.3 retrospective—build/test/chore/ci/docsare valid;tools(...)is rejected by the lefthook commit-msg hook). Mandatory even when nothing felt painful. If genuinely no gaps, document briefly: “Retrospective 05.3: no tooling gaps”. Update this subsection’sstatusin section frontmatter tocomplete. -
/sync-claudesection-close doc sync — verify Claude artifacts across all section commits. Map changed crates to rules files, check CLAUDE.md, canon.md. Fix drift NOW. -
Repo hygiene check — run
diagnostics/repo-hygiene.sh --checkand clean any detected temp files.
05.R Third Party Review Findings
- None.
05.N Completion Checklist
-
is_readonly_derived()added toDerivedTrait -
memory(read)applied to derived Eq/Comparable/Hashable functions - GVN test: repeated derived method call is eliminated
- Section 09 explicitly owns 05.2/05.3 without duplicate infrastructure in Section 05
-
timeout 150 ./test-all.shgreen (debug AND release) - Dual-exec parity:
diagnostics/dual-exec-verify.sh tests/spec/traits/derive/passes - Plan annotation cleanup
- Plan sync — update plan metadata
-
/tpr-reviewpassed -
/impl-hygiene-reviewpassed -
/improve-toolingretrospective completed — MANDATORY at section close, after both reviews are clean. Reflect on the section’s debugging journey (whichdiagnostics/scripts you ran, which command sequences you repeated, where you added ad-hocdbg!/tracingcalls, where output was hard to interpret) and identify any tool/log/diagnostic improvement that would have made this section materially easier OR that would help the next section touching this area. Implement every accepted improvement NOW (zero deferral) and commit each via SEPARATE/commit-push. The retrospective is mandatory even when nothing felt painful — that is exactly when blind spots accumulate. See.claude/skills/improve-tooling/SKILL.md“Retrospective Mode” for the full protocol.
Exit Criteria: Derived methods are marked memory(read). LLVM can CSE repeated derived calls. The plan cleanly hands ARC-level reflexive/boolean peepholes to Section 09 without duplicated pass infrastructure. Zero regressions.