Skip to content

Forward Optional projections after final SSA#29543

Open
1sgtpepper wants to merge 12 commits into
ProvableHQ:masterfrom
1sgtpepper:fix/29343-ssa-const-prop-alias-v11
Open

Forward Optional projections after final SSA#29543
1sgtpepper wants to merge 12 commits into
ProvableHQ:masterfrom
1sgtpepper:fix/29343-ssa-const-prop-alias-v11

Conversation

@1sgtpepper

@1sgtpepper 1sgtpepper commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Part of #29343.

Removes redundant lowered Optional wrappers in the post-codegen peephole pass. The rewrites require compiler-generated identity, exact operands, and safe use counts; differing fallbacks retain both selections.

Test Plan

  • compiler and execution coverage for zero, nonzero, input, and dynamic-key fallbacks
  • negative coverage for lookalikes, mismatched conditions, and escaping values
  • CircleCI

@1sgtpepper
1sgtpepper marked this pull request as ready for review June 22, 2026 03:24
@1sgtpepper
1sgtpepper force-pushed the fix/29343-ssa-const-prop-alias-v11 branch from 26df0e7 to 7338a73 Compare June 26, 2026 12:31
@1sgtpepper
1sgtpepper marked this pull request as draft June 29, 2026 05:18
@1sgtpepper
1sgtpepper force-pushed the fix/29343-ssa-const-prop-alias-v11 branch 23 times, most recently from c3a7edc to d5e58cb Compare June 30, 2026 00:41
@1sgtpepper 1sgtpepper changed the title Forward aliased aggregate fields in SSA const propagation Erase Optional materialization for guarded storage-vector gets Jun 30, 2026
@1sgtpepper
1sgtpepper force-pushed the fix/29343-ssa-const-prop-alias-v11 branch from 68d9193 to c08dd69 Compare July 2, 2026 03:24
@1sgtpepper
1sgtpepper force-pushed the fix/29343-ssa-const-prop-alias-v11 branch 8 times, most recently from 7c18038 to 54abe4b Compare July 2, 2026 11:28
@1sgtpepper
1sgtpepper marked this pull request as ready for review July 2, 2026 12:05
@1sgtpepper
1sgtpepper force-pushed the fix/29343-ssa-const-prop-alias-v11 branch from 54abe4b to f3affcd Compare July 13, 2026 23:05
@1sgtpepper
1sgtpepper marked this pull request as draft July 15, 2026 12:19
@1sgtpepper
1sgtpepper force-pushed the fix/29343-ssa-const-prop-alias-v11 branch from f3affcd to e83479e Compare July 16, 2026 00:47
@1sgtpepper 1sgtpepper changed the title Erase Optional materialization for guarded storage-vector gets Forward Optional projections after final SSA Jul 16, 2026
@1sgtpepper
1sgtpepper force-pushed the fix/29343-ssa-const-prop-alias-v11 branch from e83479e to 9172202 Compare July 16, 2026 04:40
@1sgtpepper

Copy link
Copy Markdown
Contributor Author

@mohammadfawaz I kept alias forwarding limited to generated Optionals because enabling it in the normal const-prop path changed 35 unrelated expectations. Would you keep it narrow here too?

@IGI-111 IGI-111 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.

I don't really like this solution, having partial application of some pass using some new syntax is confusing and error prone. We don't really know what has or hasn't been lowered at a given point.

I'd much rather we delay all options lowering and handle it in the intermediary passes if necessary or (and this is probably preferred) have a late optimization pass that notices and simplifies the pattern reliably.

@1sgtpepper

Copy link
Copy Markdown
Contributor Author

Yeah, that makes sense. The partial const-prop entry point is awkward, and one
concrete issue is that it only
handles finalize contexts—the same shape still shows up in
views.
So I agree this should not stay as a special AST subphase.

The late peephole sounds like the better fit. I'd keep OptionLowering where it is,
carry generated-Optional provenance through codegen, and match the exact
ternary/cast/ternary sequence there. That keeps imported Optional__... structs and
unrelated casts out of the rule. Delaying OptionLowering looks considerably larger
because flattening and codegen already assume the lowered representation. Is that
roughly the direction you had in mind?

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