Skip to content

LoopElim should be a phase, not built in into the partial evaluator#748

Merged
MikaelMayer merged 13 commits intomainfrom
issue-747-loopelim-should-be-a-phase-not-built-in
Apr 4, 2026
Merged

LoopElim should be a phase, not built in into the partial evaluator#748
MikaelMayer merged 13 commits intomainfrom
issue-747-loopelim-should-be-a-phase-not-built-in

Conversation

@MikaelMayer
Copy link
Copy Markdown
Contributor

@MikaelMayer MikaelMayer commented Apr 3, 2026

Loop elimination was tightly coupled with the partial evaluator (Procedure.eval), making it harder to prove soundness, compose with other optimizations, or replace with alternative strategies (e.g., loop unrolling).

This PR extracts loop elimination into a standalone program transformation phase (Core.loopElim) that runs after other transforms but before type checking and partial evaluation. The phase is integrated into the PipelinePhase system so that:

  • loopElimPipelinePhase.transform performs the actual loop elimination
  • loopElimPipelinePhase.phase.getValidation demotes SAT models to unknown for obligations whose path includes loop-elimination labels, since loop elimination is an over-approximation
  • The transform and its soundness annotation are coupled in the same PipelinePhase

Both verification paths (all procedures and targeted verification) now use the same corePipelinePhases list to run transforms, eliminating duplicated manual transform logic. Each phase determines its own applicability via the phase list construction.

The partial evaluator no longer needs to know about loops — it expects them to be eliminated beforehand.

All existing tests pass.

Fixes #747

…uator (#747)

Remove loop elimination from Procedure.eval (the partial evaluator) and
add it as an independent program transformation phase that runs after
other transforms (PrecondElim, CallElim) but before type checking and
partial evaluation.

- Remove removeLoopsM call and LoopElim import from ProcedureEval.lean
- Add Core.loopElim function in Verifier.lean as a reusable helper
- Call loopElim in Core.verify before typeCheckAndPartialEval
- Call loopElim in MetaVerifier.genVCs for the MetaVerifier path
- Delegate SimpleAPI.loopElimUsingContract to Core.loopElim
MikaelMayer

This comment was marked as resolved.

Moving loop elimination out of ProcedureEval removed the transitive
public import chain that made PipelinePhase types available in
Verifier.lean. Add a direct public import to restore access to
AbstractedPhase, PipelinePhase, and related definitions.
@MikaelMayer

This comment was marked as resolved.

loopElimPipelinePhase now performs the actual loop elimination
transform instead of being an identity. This ensures the pipeline
phase system properly tracks that loop elimination is an
over-approximation that should demote SAT models to unknown.

- Moved loopElim to LoopElim.lean alongside its pipeline phase
- Updated loopElimPipelinePhase.transform to use the real transform
- Integrated loop elimination into both verify code paths via pipeline
- Removed the separate loopElim step that ran outside the pipeline
@MikaelMayer

This comment was marked as resolved.

@MikaelMayer

This comment was marked as resolved.

Comment thread Strata/Languages/Core/Verifier.lean Outdated
Comment thread Strata/Languages/Core/Verifier.lean Outdated
Both the none and some procs paths now use corePipelinePhases to run
transforms, eliminating the duplicated manual transform logic.
callElimPipelinePhase is conditional on procs being provided.
Removed outdated comment justifying loop elimination placement.
@MikaelMayer MikaelMayer marked this pull request as ready for review April 3, 2026 21:02
@MikaelMayer MikaelMayer requested a review from a team April 3, 2026 21:02
@MikaelMayer MikaelMayer enabled auto-merge April 3, 2026 21:02
Copy link
Copy Markdown
Contributor

@atomb atomb left a comment

Choose a reason for hiding this comment

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

Nice improvement! One small nit: could you change the title of the PR to state what it changes rather than what the problem used to be? The current title makes it look like an issue rather than a PR.

Comment thread Strata/Transform/LoopElim.lean
@MikaelMayer MikaelMayer added this pull request to the merge queue Apr 4, 2026
Merged via the queue into main with commit e72ef17 Apr 4, 2026
15 checks passed
@MikaelMayer MikaelMayer deleted the issue-747-loopelim-should-be-a-phase-not-built-in branch April 4, 2026 04:30
@MikaelMayer
Copy link
Copy Markdown
Contributor Author

Nice improvement! One small nit: could you change the title of the PR to state what it changes rather than what the problem used to be? The current title makes it look like an issue rather than a PR.

That title made me uneasy but thanks for pointing out why. I'll be more careful next time. Feel free next time to deactivate auto-merge so that I can react accordingly.

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.

LoopElim should be a phase, not built in into the partial evaluator.

3 participants