Skip to content

perf: keep int-stable accumulator as i32 through fp-divide + |0 rounding (the image-conv kernel pattern) #49

Closed

Description

@proggeramlug

Context

Follow-up to #47 + #48. Post-fix image_conv wall time is 2142 ms vs Zig 246 ms (8.7×). Disassembly shows the remaining hot inner loop is still fp:

0x1008  fmadd d3, d9, d0, d8   ; rAcc += byte * kernel, in doubles
0x1010  ldr   d4, [sp, #0xe8]  ; index reload from stack
0x1014  fadd  d4, d4, d0       ; index += 1.0 (!)
0x1028  fcvtzs x8, d0          ; fp → int for bounds check

The pattern that currently taints

let rAcc = 0;
for (...) {
  rAcc += src[idx] * k;         // all operands fit in i16;
                                // sum of 25 fits in i18
}
dst[out] = clampU8((rAcc / KSUM) | 0);  // ← fp divide + truncate

Perry's int-spec (landed in #48) correctly says "rAcc can't be int because it flows into /". That's true for rAcc / KSUM, which is fp. But the | 0 right after the divide means the fp result is immediately truncated back to int — so fp is only a rounding step, not a semantic that needs i32→f64 conversion on every accumulate.

Proposal

Extend the int-stability analysis to recognize this exit pattern:

acc += int_expr        (n times)
result = (acc / C) | 0  where C is an int32 constant

When acc is otherwise int-stable (feeds only this pattern), keep acc as i32 inside the loop and emit the divide/round at the exit:

sdiv w_result, w_acc, w_C     ; or a mul-by-reciprocal reduction

…instead of:

scvtf d_acc, w_acc
fdiv  d_acc, d_acc, d_C
fcvtzs w_result, d_acc

Impact

Image convolution, box blur, Gaussian blur, matrix-weighted kernels, colorspace conversion, IIR filters, weighted histogram, fixed-point DSP — every signal processing idiom that uses integer weights with a final rounding divide. For honest_bench/workloads/3_image_convolution/perry/image_conv.ts specifically, would remove the per-sample fmadd d3, d9, d0, d8 round-trip inside the 25-wide kernel accumulation.

Projected for image_conv: 2142 ms → sub-1000 ms (3-4× of Zig's 246 ms), in combination with #[const-array-inlining follow-up filed separately].

Edge cases to bail on

  • Divide by a non-constant
  • Divide by 0 (wouldn't matter for | 0 but unclear semantics)
  • The accumulator's loop sum can exceed i32::MAX — analysis has to bound the worst-case sum (at compile time, from the known loop trip count + operand bounds, or conservatively leave it as fp)

Repro for verification

benchmarks/honest_bench/workloads/3_image_convolution/perry/image_conv.ts — the blur()-equivalent module body.

Activity

  1. proggeramlug commented on Apr 15, 2026

    @proggeramlug
    ContributorAuthor

    Closed by b492bff (v0.5.40).

    collect_integer_locals was extended to recognize the accumulator pattern via fixed-point iteration: LocalGet(id) is int-producing when id is itself int-stable, Uint8ArrayGet/BufferIndexGet always are, and Add/Sub/Mul are when both operands are.

    The LocalSet lowering gained a fast path: when the target has an i32 slot and every leaf of the rhs is int-sourced, the whole rhs is emitted as a short chain of add/sub/mul i32 into the i32 slot directly (one sitofp maintains the parallel double slot for non-int readers). Skips the sitofp → fadd → fptosi round-trip on every iter of acc += byte * k.

    Tight sum-of-bytes benchmark (1M × 100 iters): 272ms → 63ms (-77%).

    image_conv specifically is unchanged from 1.98s because KERNEL[ky+2][kx+2] still lowers as nested IndexGet — the k operand isn't int-lowerable until #50 lands. The two combined will hit the 400–500ms projection.

  2. added a commit that references this issue on Apr 16, 2026
    b492bff
  3. added a commit that references this issue on Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions