Skip to content

Mir dedupe#1465

Open
sbillig wants to merge 4 commits into
argotorg:masterfrom
sbillig:mir-dedupe
Open

Mir dedupe#1465
sbillig wants to merge 4 commits into
argotorg:masterfrom
sbillig:mir-dedupe

Conversation

@sbillig

@sbillig sbillig commented May 22, 2026

Copy link
Copy Markdown
Collaborator
  • Structural layout dedup
    Drop source_ty from struct/array/enum layouts and name from enum variants, so structurally-identical layouts collapse to a single LayoutId. Variant selection is by index, so the dropped names were cosmetic. This is the bulk of the snapshot churn (layouts/variants now render as struct/N, variant_N, etc.).
  • Canonical resolved-trait-method instances
    When a trait method resolves to a concrete impl, build a canonical ImplEnv (impl scope, single resolving witness, no caller assumptions) and cap method args at the impl's generic-param count, so the same resolved method dedupes across callers regardless of call-site context.

Also includes a small refactor (shared ImplEnv::for_resolved_trait_method constructor, simplified const_lowering_ty) and a regression fixture confirming trait default bodies reached through a generic param receiver still resolve under the canonical env.

Practical impact is faster compile times for large projects:
(fe test total runtime)
bountiful: 13.38s -> 4.61s
zk-kit: 58.08s -> 45.40s
chz: 17.37s -> 16.05s

@sbillig

sbillig commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator Author

Will merge this after 26.2. I think this will also require some inliner tweaks in sonatina to avoid gas regression.

@micahscopes micahscopes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

agent:

Tiny one: const_ref.rs stores the raw call-site callable.trait_inst() as the
canonical witness while classify.rs uses the normalized concrete_inst, so the
two paths can produce spelling-dependent key splits — missed dedupe only, not a
correctness issue.

Comment thread crates/mir/src/runtime/lower/classify.rs Outdated
@sbillig

sbillig commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

agent:

Tiny one: const_ref.rs stores the raw call-site callable.trait_inst() as the
canonical witness while classify.rs uses the normalized concrete_inst, so the
two paths can produce spelling-dependent key splits — missed dedupe only, not a
correctness issue.

I think this is fine, and changing it would maybe be less correct.

@sbillig

sbillig commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Will merge this after 26.2. I think this will also require some inliner tweaks in sonatina to avoid gas regression.

No inliner tweaks required. This branch provides a small gas benefit on the rosetta suite relative to master, with no regressions.

  master:      6,805,266
  mir-dedupe:  6,794,563
  delta:         -10,703 (-0.16%)

@sbillig sbillig requested a review from micahscopes June 26, 2026 22:27
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.

2 participants