Skip to content

(feat): Added an optimization dump for each phase under a special log target#9873

Open
eytan-starkware wants to merge 2 commits intomainfrom
eytan_graphite/_feat_added_an_optimization_dump_for_each_phase_under_a_special_log_target
Open

(feat): Added an optimization dump for each phase under a special log target#9873
eytan-starkware wants to merge 2 commits intomainfrom
eytan_graphite/_feat_added_an_optimization_dump_for_each_phase_under_a_special_log_target

Conversation

@eytan-starkware
Copy link
Copy Markdown
Contributor

@eytan-starkware eytan-starkware commented Apr 23, 2026

Summary

Adds tracing instrumentation to the optimization pipeline that logs the lowered IR before any optimization phases run and after each individual phase completes. Logs are emitted at the info level under the optimization_dump target, including the function's full path and the formatted lowered representation.


Type of change

Please check one:

  • Bug fix (fixes incorrect behavior)
  • New feature
  • Performance improvement
  • Documentation change with concrete technical impact
  • Style, wording, formatting, or typo-only change

⚠️ Note:
To keep maintainer workload sustainable, we generally do not accept PRs that
are only minor wording, grammar, formatting, or style changes.
Such PRs may be closed without detailed review.


Why is this change needed?

Without visibility into the intermediate lowered IR states between optimization phases, it is difficult to debug incorrect or unexpected transformations introduced by a specific phase. This change makes it possible to trace exactly how the lowered IR evolves through each optimization step.


What was the behavior or documentation before?

The optimization pipeline applied each phase sequentially with no observable output about the intermediate IR states.


What is the behavior or documentation after?

When tracing is enabled at the info level for the optimization_dump target, the lowered IR is logged before any phases run and after each phase completes, labeled with the function's full path and the phase name.


Related issue or discussion (if any)


Additional context

The logs can be selectively enabled using a tracing subscriber filtered to the optimization_dump target, making this a low-overhead addition when tracing is not active.

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 23, 2026

PR Summary

Low Risk
Low risk: adds tracing::trace! instrumentation to the optimization pipeline without changing optimization behavior. Main concern is potential log/formatting overhead when the optimization_dump target is enabled at trace level.

Overview
Adds an optimization dump trace stream in the lowering optimization pipeline.

When applying an OptimizationStrategy, it now logs the formatted lowered IR once before any phases run and again after each phase, under the optimization_dump tracing target, annotated with the function path and phase name.

Reviewed by Cursor Bugbot for commit 7b2c047. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9b96bd2d82

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/cairo-lang-lowering/src/optimizations/strategy.rs Outdated
Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

@orizi made 2 comments.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on eytan-starkware and TomerStarkware).


crates/cairo-lang-lowering/src/optimizations/strategy.rs line 144 at r1 (raw file):

    ) -> Maybe<()> {
        let fmt = crate::fmt::LoweredFormatter::new(db, &lowered.variables);
        tracing::info!(

Suggestion:

        tracing::trace!(

crates/cairo-lang-lowering/src/optimizations/strategy.rs line 154 at r1 (raw file):

            phase.apply(db, function, lowered)?;
            let fmt = crate::fmt::LoweredFormatter::new(db, &lowered.variables);
            tracing::info!(

Suggestion:

            tracing::trace!(

@eytan-starkware eytan-starkware force-pushed the eytan_graphite/_feat_added_an_optimization_dump_for_each_phase_under_a_special_log_target branch from 9b96bd2 to f626392 Compare May 3, 2026 10:59
@eytan-starkware eytan-starkware force-pushed the eytan_graphite/_feat_merging_analysis_tests branch from aeb10da to 5d2a519 Compare May 3, 2026 10:59
Copy link
Copy Markdown
Contributor Author

@eytan-starkware eytan-starkware left a comment

Choose a reason for hiding this comment

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

@eytan-starkware made 3 comments.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on orizi and TomerStarkware).


crates/cairo-lang-lowering/src/optimizations/strategy.rs line 144 at r1 (raw file):

    ) -> Maybe<()> {
        let fmt = crate::fmt::LoweredFormatter::new(db, &lowered.variables);
        tracing::info!(

Done.


crates/cairo-lang-lowering/src/optimizations/strategy.rs line 154 at r1 (raw file):

            phase.apply(db, function, lowered)?;
            let fmt = crate::fmt::LoweredFormatter::new(db, &lowered.variables);
            tracing::info!(

Done.

Comment thread crates/cairo-lang-lowering/src/optimizations/strategy.rs Outdated
@eytan-starkware eytan-starkware changed the base branch from eytan_graphite/_feat_merging_analysis_tests to graphite-base/9873 May 3, 2026 11:01
Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

:lgtm:

@orizi reviewed 1 file and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on TomerStarkware).

@eytan-starkware eytan-starkware force-pushed the eytan_graphite/_feat_added_an_optimization_dump_for_each_phase_under_a_special_log_target branch from f626392 to 5689e86 Compare May 3, 2026 12:03
@eytan-starkware eytan-starkware changed the base branch from graphite-base/9873 to eytan_graphite/_feat_merging_analysis_tests May 3, 2026 12:03
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5689e86. Configure here.

Comment thread crates/cairo-lang-lowering/src/optimizations/strategy.rs
@eytan-starkware eytan-starkware force-pushed the eytan_graphite/_feat_merging_analysis_tests branch from 58b48e9 to 5444b1b Compare May 3, 2026 12:10
@eytan-starkware eytan-starkware force-pushed the eytan_graphite/_feat_added_an_optimization_dump_for_each_phase_under_a_special_log_target branch from 5689e86 to 7b2c047 Compare May 3, 2026 12:10
@eytan-starkware eytan-starkware changed the base branch from eytan_graphite/_feat_merging_analysis_tests to main May 3, 2026 13:00
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.

3 participants