Skip to content

fix: beta reduction no longer duplicates a non-trivial argument (#167) - #191

Merged
Unisay merged 1 commit into
mainfrom
issue-167/beta-reduce-let-nontrivial-args
Jul 6, 2026
Merged

fix: beta reduction no longer duplicates a non-trivial argument (#167)#191
Unisay merged 1 commit into
mainfrom
issue-167/beta-reduce-let-nontrivial-args

Conversation

@Unisay

@Unisay Unisay commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Fixes #167.

Problem

betaReduce rewrote App (Abs (ParamNamed x) body) r by substituting r into every occurrence of x, without looking at the shape of r or the number of occurrences. In a strict language that duplicates evaluation: (\x -> ... x ... x ...) (g y) became ... (g y) ... (g y) ..., so the work of g y ran once per occurrence.

The path is live. The inliner puts a lambda into call-head position for used-once bindings and for @inline always, so the better the inliner gets, the more of these redexes betaReduce sees.

Fix

Apply GHC's inlining rule, the same one inlineLocalBinding already uses:

  • Substitute directly when the argument is trivial (a Ref, a literal, or @inline always, where re-evaluation is free) or the parameter is used at most once.
  • Otherwise rewrite the redex to let x = r in body and leave the inline-or-not decision to inlineLocalBindings.

The guard in betaReduce is deliberately the mirror of the one in inlineLocalBinding (isInlinableExpr r || occurrences <= 1). That is what makes the rewrite converge in one bottom-up pass: the Let that betaReduce produces is exactly the one inlineLocalBindings then declines to inline, so nothing ping-pongs. Both sites reference a shared Note [Beta reduction and local inlining share an inlining guard] that spells this out.

The zero-occurrence case keeps today's behaviour: countFreeRef x body <= 1 covers it, so an unused non-trivial argument is still discarded. The Exception-in-a-dropped-argument edge follows the evaluation-order policy the pipeline already declares for let-bound values (the FloatIn note: discardable and reorderable once nothing observes them).

Tests

TDD, in IR/Optimizer/Spec.hs under a new beta reduction does not duplicate work (#167) group, all driven through optimizedExpression:

  • The regression case: a non-trivial argument (g 1) with the parameter used twice. Before the fix it optimised to Eq (g 1) (g 1) (the duplication); it now optimises to let x = g 1 in Eq x x. I confirmed this case is red before the fix and green after.
  • Three pins that are green both before and after, guarding the substitute path: a trivial Ref used twice still substitutes; a non-trivial argument used exactly once still substitutes; a non-trivial argument never used is still discarded.

Golden churn

Structural goldens (golden.ir / golden.lua) move in the expected direction: repeatedly-used non-trivial arguments (dictionary expressions) are now shared through a local binding instead of pasted at each use site. The hand-verified eval/golden.txt oracles are unchanged, so runtime behaviour is preserved.

@Unisay
Unisay requested a review from Copilot July 6, 2026 14:04
@Unisay Unisay self-assigned this Jul 6, 2026
@Unisay
Unisay marked this pull request as ready for review July 6, 2026 14:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a performance defect in the IR optimizer’s betaReduce rewrite: it no longer duplicates evaluation work by blindly substituting non-trivial arguments into multi-use parameters. Instead, it mirrors the existing inlineLocalBinding heuristic: substitute when the argument is inlinable/trivial or the parameter occurs at most once; otherwise, rewrite the redex into a let binding to share the argument.

Changes:

  • Update betaReduce to avoid duplicating non-trivial arguments by introducing a let when the parameter is used more than once.
  • Add a focused regression test group for issue #167, pinning both the new sharing behavior and the existing substitution/discard behavior.
  • Refresh structural golden outputs (golden.ir / golden.lua) to reflect the new sharing introduced by the optimizer.

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.

Show a summary per file
File Description
lib/Language/PureScript/Backend/IR/Optimizer.hs Changes betaReduce to substitute only when safe; otherwise let-binds to avoid duplicated work.
test/Language/PureScript/Backend/IR/Optimizer/Spec.hs Adds regression tests asserting beta-reduction no longer duplicates non-trivial arguments.
test/ps/output/Golden.TailRecM2Shadow.Test/golden.ir Updates structural golden IR after optimizer change (more sharing via let).
test/ps/output/Golden.TailRecM2Shadow.Test/golden.lua Updates structural golden Lua after optimizer change (more sharing via locals).
test/ps/output/Golden.StringCodePoints.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.StringCodePoints.Test/golden.lua Updates structural golden Lua after optimizer change.
test/ps/output/Golden.MaybeChain.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.MaybeChain.Test/golden.lua Updates structural golden Lua after optimizer change.
test/ps/output/Golden.LongWriterBind.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.LongWriterBind.Test/golden.lua Updates structural golden Lua after optimizer change.
test/ps/output/Golden.LongStateBind.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.LongStateBind.Test/golden.lua Updates structural golden Lua after optimizer change.
test/ps/output/Golden.LongStackBind.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.LongStackBind.Test/golden.lua Updates structural golden Lua after optimizer change.
test/ps/output/Golden.LongReaderBind.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.LongReaderBind.Test/golden.lua Updates structural golden Lua after optimizer change.
test/ps/output/Golden.LongMaybeBind.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.LongMaybeBind.Test/golden.lua Updates structural golden Lua after optimizer change.
test/ps/output/Golden.LongExceptBind.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.LongExceptBind.Test/golden.lua Updates structural golden Lua after optimizer change.
test/ps/output/Golden.LongEitherBind.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.LongEitherBind.Test/golden.lua Updates structural golden Lua after optimizer change.
test/ps/output/Golden.LongBindFlipped.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.LongApplyChain.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.Issue37.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.Issue37.Test/golden.lua Updates structural golden Lua after optimizer change.
test/ps/output/Golden.GenericEqTwoTypes.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.GenericEqTwoTypes.Test/golden.lua Updates structural golden Lua after optimizer change.
test/ps/output/Golden.ArrayOfUnits.Test/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.ArrayOfUnits.Test/golden.lua Updates structural golden Lua after optimizer change.
test/ps/output/Golden.Annotations.M2/golden.ir Updates structural golden IR after optimizer change.
test/ps/output/Golden.Annotations.M2/golden.lua Updates structural golden Lua after optimizer change.

betaReduce rewrote App (Abs (ParamNamed x) body) r by substituting r into
every occurrence of x, ignoring the shape of r and the occurrence count. In
a strict language that duplicates work: (\x -> ... x ... x ...) (g y) became
... (g y) ... (g y) ..., so g y ran once per occurrence. The inliner puts
lambdas into call-head position (used-once bindings, @inline always), so the
better the inliner gets, the more such redexes this rule sees.

Apply GHC's rule, mirroring inlineLocalBinding: substitute only when the
argument is trivial (Ref / literal / @inline-always) or the parameter is used
at most once; otherwise rewrite the redex to `let x = r in body` and leave the
inline-or-not decision to inlineLocalBindings. The guard is deliberately the
mirror of inlineLocalBinding's, so the Let produced here is exactly the one
inlineLocalBindings then declines to inline and the rewrite converges in one
bottom-up pass. The zero-occurrence discard is unchanged (<= 1 covers it).

Tests (IR/Optimizer/Spec.hs, via optimizedExpression): the regression case
(non-trivial argument used twice) is red before this change -- it optimised to
Eq (g 1) (g 1) -- and green after (let x = g 1 in Eq x x). Three pins guard
the substitute path: a trivial ref used twice, a non-trivial argument used
once, and a non-trivial argument never used.

Structural goldens move toward sharing repeatedly-used non-trivial
subexpressions through a local binding instead of pasting them at each use
site; the hand-verified eval/golden.txt oracles are unchanged.
@Unisay
Unisay force-pushed the issue-167/beta-reduce-let-nontrivial-args branch from 1604e0c to b75f69f Compare July 6, 2026 14:13
@Unisay
Unisay merged commit 0820dde into main Jul 6, 2026
2 checks passed
@Unisay
Unisay deleted the issue-167/beta-reduce-let-nontrivial-args branch July 6, 2026 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

betaReduce substitutes a non-trivial argument into every occurrence of the parameter, multiplying work

2 participants