Skip to content

fix(ir): keep a bare-Ref alias to an @inline always binding shared (#171) - #257

Merged
Unisay merged 2 commits into
mainfrom
issue-171/keep-alias-to-always-binding
Jul 12, 2026
Merged

fix(ir): keep a bare-Ref alias to an @inline always binding shared (#171)#257
Unisay merged 2 commits into
mainfrom
issue-171/keep-alias-to-always-binding

Conversation

@Unisay

@Unisay Unisay commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Closes #171.

Problem

Two individually sound inliner rules compose badly. The top-level inliner dissolves any bare-Ref binding into its use sites (a reference replacing a reference is free), and an @inline always binding pastes its body into every site that references it. Chain them and the alias multiplies the paste: Main.add = Ref Data.Semiring.intAdd used at two sites dissolves first, taking intAdd's reference count from one to two, and then Always fires at both. The result is two copies of the body where one shared alias binding was the better materialization point on both size and speed.

Since #248 the linker no longer auto-annotates plain foreign accessors, so the auto-always shape that triggers this today is a ForeignLift-lifted body: a lambda marked always by the lift so its call sites beta-reduce. An alias to such a binding used at several value positions (exports, record fields, arguments) gets the whole lambda duplicated per site, with no beta-reduction to pay for it.

What changed

withBinding now decides through topLevelInlinable, which diverges from the shared isInlinableExpr twice:

  • The bare-Ref tier declines when the target name carries @inline always (policyAlways, a new InlinePolicy field). The alias survives, the target's body pastes into it once, and every alias site keeps referencing the single copy. Saturated call sites still reduce: the call-site inliner reads the alias's lambda RHS out of the inline environment as usual.
  • The Always decision itself is keyed by name rather than by the RHS root annotation, for the reason every other directive already is: a rewrite can drop or transplant a live node's annotation, so the tree carries no reliable directive signal. Name-keying also closes the standing gap where a root annotation stripped by an earlier rewrite silently disabled an explicit always.

On top of that, annotations are now spent rather than left to drift. A whole-binding paste sheds the root annotation (mirroring the call-site paste in inlineSaturatedCall), so a binding that merely received an always-annotated body never turns unconditionally inlinable itself. That removes the last consumer that depended on an annotation surviving optimization: shareForeignAccessors used to test each read node's annotation to honor the per-site contract of an explicit always accessor pragma, and now reads the opt-out from the same name-keyed policy instead. The pragma's meaning no longer hangs on an annotation happening to survive every rewrite between the linker and the last pass.

The local rules (betaReduce, inlineLocalBinding) keep reading the root annotation directly: isInlinableExpr is unchanged there, with its structural tiers split out as isInlinableValue for the top-level predicate to reuse.

Explicit pragma contracts are preserved. A user's @inline always accessor still ends as a per-site field read even through an alias: the read pasted into the alias is a cheap projection, so the Deref tier dissolves the alias on the next round, and shareForeignAccessors skips the copies by name. That is the per-site contract #248 documented. The scope note in #171 confines the defect to the auto-annotation interaction, and that is what the fix targets.

Verification

  • New unit test in Optimizer/Spec (red before the fix, green after): a bare-Ref alias to an always-annotated unary lambda, referenced by two exports, survives as the single binding, both exports keep referencing it, and the body materialized into the alias carries no annotation.
  • Regression pins around the new surface: a use-once alias to an always binding still dissolves (the veto guards multi-use aliases only), a fold that drops the root annotation no longer disables an explicit always (the @inline never mirror from @inline never is silently ignored and does not prevent inlining #131), and shareForeignAccessors re-binds annotated reads of an undirected name while normalizing the inserted binding's annotation slot.
  • The Revisit the foreign-accessor @inline always default: stage-2 promotion made the shared form free #248 suite pins the name-keyed opt-out: a twice-referenced accessor annotated @inline always still dissolves per site even though the pasted reads now carry no annotation.
  • Golden corpus (after rebasing onto the Consume prelude v7.3.1: share the unit singleton (@inline unit never) #258 package-set repoint): 6 golden.ir files churn, every changed line a Just Always printing as Nothing on a pasted node. Every golden.lua and every eval oracle is byte-identical.
  • cabal test all green on the default seed plus seeds 42, 1337 and 999999 (the optimizer was touched); hlint clean; fourmolu applied.

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 an IR optimizer inlining interaction where dissolving a bare Ref alias could inadvertently multiply @inline always inlining across multiple use sites (issue #171), increasing code size and potentially slowing generated Lua.

Changes:

  • Extend the optimizer’s InlinePolicy to track @inline always directives by binding name (policyAlways) and use that for top-level inlining decisions.
  • Adjust the top-level inliner to not dissolve a bare-Ref alias when it points at an @inline always binding, preserving the alias as a single materialization point.
  • Add a regression unit test covering the multi-export alias case, plus a changelog fragment.

Reviewed changes

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

File Description
lib/Language/PureScript/Backend/IR/Optimizer.hs Introduces policyAlways and updates top-level inlining to keep bare-Ref aliases to always names while still enforcing always by name.
lib/Language/PureScript/Backend/IR/Inliner.hs Updates inline-policy documentation to reflect name-keyed always handling and the alias preservation rule.
test/Language/PureScript/Backend/IR/Optimizer/Spec.hs Adds a regression test asserting the alias survives and both exports still reference it.
changelog.d/20260712_190000_unisay_alias_to_always_binding.md Documents the fix in the changelog fragment format.

@Unisay Unisay self-assigned this Jul 12, 2026
Unisay added 2 commits July 12, 2026 21:38
…share opt-out (#171)

A whole-binding paste no longer lets the root annotation ride into the
host binding: the pasted copy sheds it, mirroring the call-site paste in
inlineSaturatedCall. With that, node annotations carry no directive
weight once optimization starts rewriting the tree, so
shareForeignAccessors now reads its @inline always opt-out from the
name-keyed InlinePolicy instead of testing each read node's surviving
annotation — the per-site contract of an explicit always pragma no
longer depends on an annotation happening to survive every rewrite.

Structural golden churn is annotation display only: pasted reads print
Nothing where they printed Just Always. Generated Lua and eval oracles
are byte-identical.
…e share pass's annotation blindness (#171)

Three regression pins around the #171 surface: a use-once alias to an
@inline always binding still dissolves (the veto guards multi-use
aliases only), a fold that drops the root annotation no longer disables
an explicit always (the @inline never mirror), and shareForeignAccessors
re-binds annotated reads of an undirected name, normalizing the inserted
binding's annotation slot.
@Unisay
Unisay force-pushed the issue-171/keep-alias-to-always-binding branch from 1109dd8 to cbb79d3 Compare July 12, 2026 19:40
@Unisay
Unisay requested a review from Copilot July 12, 2026 19:43

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

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

@Unisay
Unisay merged commit c99ae95 into main Jul 12, 2026
3 checks passed
@Unisay
Unisay deleted the issue-171/keep-alias-to-always-binding branch July 12, 2026 19:48
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.

Dissolving a bare-Ref alias to an @inline always binding multiplies the inlined body across all use sites

2 participants