Skip to content

Split assertionprop.cpp into integralrange.cpp and vnbasedmorph.cpp#124394

Closed
Copilot wants to merge 3 commits intomainfrom
copilot/split-assertionprop-into-files
Closed

Split assertionprop.cpp into integralrange.cpp and vnbasedmorph.cpp#124394
Copilot wants to merge 3 commits intomainfrom
copilot/split-assertionprop-into-files

Conversation

Copy link
Contributor

Copilot AI commented Feb 13, 2026

Description

Split assertionprop.cpp (6321 lines) into three focused compilation units for better code organization.

Changes

  • integralrange.cpp (596 lines): All IntegralRange::* methods

    • Range computation, type bounds, cast analysis, symbolic arithmetic
  • vnbasedmorph.cpp (1040 lines): VN-based optimization functions

    • optVNBasedFold* family (memset/memmove unrolling, expression folding)
    • optIsProfitableToSubstitute, optVNConstantPropOnJTrue
    • optVNAssertionPropCurStmt* assertion propagation
    • VNAssertionPropVisitorInfo struct
  • assertionprop.cpp (4722 lines): Remaining assertion propagation infrastructure

    • AssertionPropFlowCallback dataflow analysis
    • optComputeAssertionGen, optAssertionPropMain
    • Assertion tracking and propagation core

Updated CMakeLists.txt to register new compilation units.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

….cpp, and assertionprop.cpp

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
@github-actions github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 13, 2026
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

…between them

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Copilot AI changed the title [WIP] Split assertionprop.cpp into multiple files Split assertionprop.cpp into integralrange.cpp and vnbasedmorph.cpp Feb 13, 2026
Copilot AI requested a review from EgorBo February 13, 2026 18:34
@EgorBo
Copy link
Member

EgorBo commented Feb 13, 2026

PTAL @dotnet/jit-contrib no diffs (except for some spurious diffs #124371) cleanup change. I just wanted to move unrelated to AssertionProp things out of assertionprop.cpp which is too big and hard to read:

  • integralrange.cpp - 600 LOC related to IntegralRange class (used by multiple phases)
  • vnbasedmorph.cpp - VN-based morph. It's when we iterate over all trees and perform constant foldings and other optimizations purely over VNs, no usages of any assertions, so like global morph but with VN. I moved the following functions there:
Compiler::optIsProfitableToSubstitute(...)
Compiler::optVNBasedFoldExpr_Call_Memset(...)
Compiler::optVNBasedFoldExpr_Call_Memmove(...)
Compiler::optVNBasedFoldExpr_Call(...)
Compiler::optVNBasedFoldExpr(...)
Compiler::optVNBasedFoldConstExpr(...)
Compiler::optVNBasedFoldCurStmt
Compiler::optVNConstantPropOnJTrue(...)
Compiler::optVnNonNullPropCurStmt(...)
Compiler::optVNAssertionPropCurStmtVisitor(...)
Compiler::optVNAssertionPropCurStmt(...)

Intentionally didn't try to wrap that into a separate phase or rename anything to avoid complicating the code-review.

I recommend reviewing with whitespaces disabled.

@EgorBo EgorBo marked this pull request as ready for review February 13, 2026 19:39
@EgorBo EgorBo requested review from a team and Copilot February 13, 2026 19:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors CoreCLR JIT assertion propagation by splitting the large assertionprop.cpp compilation unit into smaller, more focused units to improve code organization and maintainability.

Changes:

  • Added integralrange.cpp containing all IntegralRange::* method implementations.
  • Added vnbasedmorph.cpp containing VN-based folding/constant-prop and related visitors/helpers.
  • Reduced assertionprop.cpp to the remaining assertion propagation infrastructure and updated CMakeLists.txt to compile the new units.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

File Description
src/coreclr/jit/vnbasedmorph.cpp New compilation unit for VN-based folding and VN-driven assertion propagation.
src/coreclr/jit/integralrange.cpp New compilation unit for IntegralRange implementation details.
src/coreclr/jit/assertionprop.cpp Removes code moved into the new compilation units; retains assertion propagation core.
src/coreclr/jit/CMakeLists.txt Registers the new .cpp files in the JIT build.

@jakobbotsch
Copy link
Member

Does git blame -CCC still work with this? I had to move things in smaller batches when I did the importer to ensure that it could track lines across the files.

@EgorBo
Copy link
Member

EgorBo commented Feb 16, 2026

Does git blame -CCC still work with this? I had to move things in smaller batches when I did the importer to ensure that it could track lines across the files.

good point, let me do it by hands to ensure that

@EgorBo EgorBo closed this Feb 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants