Merged flow based rewriting pass

[ychenfo]

• Add the merged pass
• Move the shared logic between deforest and the merged pass into PolyInstantiationRewrite
• Add dead ctor elim
• Make eta expansion support polymorphic analysis result and add manual @affine annot for testing
• Update config to use only one flowBasedOpt
• Minor fixes on opaque constraints
• Add :showPipeline for checking program changes across different pass more easily

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[ychenfo]

It might be helpful to use "Hide whitespaces" to review the diff of this file because of indentation changes of a large chunk of code.

[Copilot]

🟡 Changes recommended

Critical correctness issues remain in constructor elimination, flow rewriting, eta analysis, and rewriter initialization.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

hkmc2/shared/src/main/scala/hkmc2/codegen/FlowAnalysisBasedRewrite.scala:285

• rewriteWith is a core optimizer entry point, but these defaults make omitted flags silently enable all three transformations; EtaExpansion.apply and DeadParamElim.apply rely on this implicit contract. Keep the mode selection explicit at each call site (or expose separate entry points) instead of using default arguments here.

    eta: Bool = true,
    dpe: Bool = true,
    dce: Bool = true,

hkmc2/shared/src/main/scala/hkmc2/codegen/PolyInstantiationRewrite.scala:81

• The generated specialized symbol keeps f.erasedType, although mkPolyFunCopy can remove parameters for DPE and append parameter lists for eta expansion. Consumers such as Result.erasedType use the symbol's FuncRef arity, so calls to these poly copies can expose the original arity/result metadata instead of the rewritten definition; the previous DPE implementation explicitly built a filtered specialized type. Derive the specialized erased type from the rewritten parameter lists before creating this symbol.

            f -> (
              new BlockMemberSymbol(name, Nil, true),
              new TermSymbol(Fun, N, Tree.Ident(name), erasedType = f.erasedType))

• Files reviewed: 39/39 changed files
• Comments generated: 6
• Review effort level: Lite

[ychenfo]

The same issue as the first comment, fixed.

[LPTK]

Don't create useless intermediate lists... Use reverseIterator, ++ selectedFrom (without toList), and foldLeft.

[LPTK]

[Astra] Changes requested: four reproduced correctness issues remain in constructor elimination and variadic eta expansion. Each inline comment links to its regression test and includes a standalone reproducer. The tests and golden outputs are retained with :fixme in commit d05a78f23. Validation performed during the review: ctest and hkmc2AllTests/test passed with the known failures recorded, and each of the four original reproducers succeeded with flowBasedOpt disabled.

[LPTK]

[Astra] [P1] Preserve arguments observed by constructor bodies

Discarding a constructor's result does not make its arguments dead. The simple-constructor check protects Consumer itself, but not the nested Observed allocation below: this becomes Consumer(undefined) and throws instead of returning 42. Constructor-body uses need to constrain the actual arguments; until that flow is modeled, the affected arguments must be retained conservatively. Leaving this as a TODO for a later PR is unsafe while constructor elimination is enabled by default.

Regression: consume(). The retained test has :fixme; omit it to reproduce the failure:

:js
:noInline
:flowBasedOpt mono
:expect 42
data class Observed(x)
class Consumer(x) with
  if x is Observed then () else throw "expected Observed"
fun consume() =
  let unused = Consumer(Observed(1))
  42
consume()

The same program returns 42 with :flowBasedOpt off.

[LPTK]

[Astra] [P1] Account for constructor arguments added by lifting

The source parameter list excludes capture parameters added by Lifter, so clsParams.zip(argsStrat) silently drops those actual arguments from the flow graph. Here the captured tuple remains live through get(), but its allocation is removed and the generated code calls new Capturing(0, undefined). Use the current IR parameter/field mapping, or conservatively constrain unmatched arguments as live.

Regression: capture(). Without its :fixme marker:

:js
:noInline
:flowBasedOpt mono
:lift
:expect [1, 2]
fun capture() =
  let captured = [1, 2]
  class Capturing(x) with
    fun get() = captured
  let instance = Capturing(0)
  instance.get()
capture()

The result is rendered as () instead of [1, 2]; disabling flowBasedOpt restores the expected result.

[LPTK]

[Astra] [P1] Treat checked casts as uses of allocations

This liveness calculation relies on consumer edges, but FlowAnalysis.processResult currently handles Cast(value, _, _) by returning the operand's flow without recording the runtime check. An allocation used only by an otherwise-unused checked cast is therefore removed. The Wasm program below changes into undefined as! Box and traps with RuntimeError: illegal cast instead of returning 42. A checked cast must keep its operand live even when its result is discarded.

Regression: first cast() case. A second case covers the same flow through an intermediate val. Without the retained :fixme marker:

:global
:wasm
:noInline
:flowBasedOpt mono
data class Box(x)
fun cast() =
  let x = Box(1)
  val unused: Box = x as Object
  42
cast()

With :flowBasedOpt off, the first case returns 42 successfully.

[LPTK]

[Astra] [P2] Preserve variadic parameters during eta expansion

EtaTargets counts a rest parameter as one parameter, but the synthesized list always has restParam = N and forwards only ordinary arguments. The newly supported @affine(1) annotation therefore allows this wrapper to silently truncate (1, 2, 3) to (1). The returned function is called exactly once, so the annotation's one-shot promise is satisfied. Preserve and spread the rest parameter, or exclude variadic targets from eta expansion.

Regression: returnRest. Without its :fixme marker:

:js
:noInline
:flowBasedOpt mono
:expect [1, 2, 3]
@affine(1) fun rest(a)(...xs) = xs
fun returnRest(a) = rest(a)
returnRest(0)(1, 2, 3)

This returns [1]; with :flowBasedOpt off, it returns [1, 2, 3].