Cleanup basic block by AndyAyersMS · Pull Request #124915 · dotnet/runtime

AndyAyersMS · 2026-02-26T18:26:15Z

Encapsulate a few of the public fields on BasicBlock, and do some related cleanup.

…/block.cpp - Fix typo in FlowEdge::addLikelihood declaration (addedLikelihod -> addedLikelihood) - Fix comment typos: 'rage' -> 'range', 'inheritWeightPecentage' -> 'inheritWeightPercentage' - Remove tautological assert (0 <= unsigned is always true) - Add const to HasTerminator() and SuccEdges() for consistency - Add missing BBF_HAS_VALUE_PROFILE, BBF_MAY_HAVE_BOUNDS_CHECKS, BBF_THROW_HELPER to dspFlags() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Remove references to removed flEdgeWeightMin/flEdgeWeightMax fields and the bounded-range edge weight model. Update to reflect that FlowEdge now stores both source and destination blocks, and uses a likelihood value. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add SetFirstStmt() setter method to complement the existing firstStmt() getter. Replace all external reads of bbStmtList with firstStmt() and all external writes with SetFirstStmt(). Internal BasicBlock methods retain direct field access. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add GetCatchTyp()/SetCatchTyp() accessor methods. Replace all external reads with GetCatchTyp() and writes with SetCatchTyp(). Internal BasicBlock methods (CloneBlockState, hasEHBoundaryIn) retain direct field access. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add CatchTypIs() following the same pattern as KindIs() to replace verbose GetCatchTyp() == comparisons. Supports single and multi-arg forms (e.g., CatchTypIs(BBCT_FINALLY, BBCT_FAULT)). Remaining GetCatchTyp() calls are intentional: they pass the value to handlerGetsXcptnObj(), switch statements, or SetCatchTyp(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

bbStkDepth was a redundant copy of bbEntryState->esStackDepth. The existing bbStackDepthOnEntry() method already returns the correct value (reading from bbEntryState, or 0 when null). Replace all uses with bbStackDepthOnEntry() and remove the field. The explicit assignment after initBBEntryState() is also removed since initBBEntryState() already sets bbEntryState->esStackDepth. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add GetEntryState()/SetEntryState() accessor methods. Replace all external reads and writes in importer.cpp. Internal BasicBlock methods (bbStackDepthOnEntry, bbSetStack, bbStackOnEntry, New) retain direct field access. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The esRefcount field was removed from EntryState long ago. Delete the stale commented-out reference to it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Replace CMK_Unknown with proper memory categories: - PendingDsc, EntryState, StackEntry[] -> CMK_ImpStack (import stack) - methodPointerInfo -> CMK_ASTNode (IR tree node data) These match the categories already used for identical allocations elsewhere in the same file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

dotnet-policy-service · 2026-02-26T18:27:21Z

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

AndyAyersMS · 2026-02-26T18:29:09Z

@dotnet/jit-contrib PTAL

The ultimate ambition here is to encapsulate all the fields so we can experiment with different implementation strategies (eg more extensive but also properly safe use of unions since many block fields are only live for parts of compilation, maybe SoA approaches for some fields, and keeping seldomly used fields in some other kind of structure).

Assuming this goes well we can then try something similar for the IR... blocks are simpler.

Copilot

Pull request overview

Encapsulates several BasicBlock internals in the CoreCLR JIT by moving previously-public fields behind accessors, and updates call sites accordingly to reduce direct field coupling.

Changes:

Move BasicBlock statement list, EH catch type, and entry-state fields to private and introduce/access accessor helpers (SetFirstStmt, GetCatchTyp/SetCatchTyp/CatchTypIs, GetEntryState/SetEntryState).
Mechanically update JIT phases/codegen/importer to use the new BasicBlock APIs instead of direct field access.
Minor cleanups: updated debug flag display strings and adjusted some allocator “CMK_*” tags for importer-related allocations.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/coreclr/jit/ssabuilder.cpp	Switches statement list manipulation to `firstStmt()`/`SetFirstStmt()` accessors.
src/coreclr/jit/rationalize.cpp	Clears statements via `SetFirstStmt(nullptr)` rather than direct field access.
src/coreclr/jit/optimizer.cpp	Updates statement-list assertions to use `firstStmt()`.
src/coreclr/jit/morph.cpp	Updates statement-list assertions to use `firstStmt()`.
src/coreclr/jit/liveness.cpp	Adjusts comment wording to avoid referencing encapsulated field name.
src/coreclr/jit/jiteh.cpp	Replaces direct `bbCatchTyp` reads/writes with `GetCatchTyp`/`SetCatchTyp`/`CatchTypIs`.
src/coreclr/jit/importer.cpp	Migrates to `SetFirstStmt`, `CatchTypIs`, `GetEntryState`/`SetEntryState`, and `bbStackDepthOnEntry()`.
src/coreclr/jit/ifconversion.cpp	Clears statements via `SetFirstStmt(nullptr)`.
src/coreclr/jit/flowgraph.cpp	Uses `SetCatchTyp` and `firstStmt()` in diagnostics/debug logic.
src/coreclr/jit/fgstmt.cpp	Updates statement insertion/removal helpers to use `SetFirstStmt`/`firstStmt()`.
src/coreclr/jit/fgopt.cpp	Updates compaction/unreachable/branch opt code to use statement and catch-type accessors.
src/coreclr/jit/fginline.cpp	Updates inline block insertion checks to use `firstStmt()`.
src/coreclr/jit/fgehopt.cpp	Updates EH transforms to use `SetCatchTyp`.
src/coreclr/jit/fgdiagnostic.cpp	Uses `GetCatchTyp`/`CatchTypIs` and `firstStmt()` for block display/checks.
src/coreclr/jit/fgbasic.cpp	Updates EH setup and block splitting to use `SetCatchTyp`/`SetFirstStmt`/`firstStmt()`.
src/coreclr/jit/copyprop.cpp	Uses `CatchTypIs` helper for filtering EH block kinds.
src/coreclr/jit/codegenxarch.cpp	Uses `GetCatchTyp`/`CatchTypIs` for funclet/catch-arg handling.
src/coreclr/jit/codegenriscv64.cpp	Uses `GetCatchTyp`/`CatchTypIs` for funclet/catch-arg handling.
src/coreclr/jit/codegenloongarch64.cpp	Uses `GetCatchTyp`/`CatchTypIs` for funclet/catch-arg handling.
src/coreclr/jit/codegenlinear.cpp	Uses `GetCatchTyp` for catch-arg handling in linear codegen.
src/coreclr/jit/codegenarmarch.cpp	Uses `GetCatchTyp` for catch-arg handling.
src/coreclr/jit/codegenarm64.cpp	Uses `CatchTypIs` for funclet prolog decisions.
src/coreclr/jit/codegenarm.cpp	Uses `CatchTypIs` for funclet prolog decisions.
src/coreclr/jit/block.h	Makes `bbStmtList`/`bbCatchTyp`/`bbEntryState` private and adds accessor APIs; minor signature const-ness tweaks.
src/coreclr/jit/block.cpp	Updates debug flag display names and removes `bbStkDepth` cloning; FlowEdge likelihood naming fix.
src/coreclr/jit/async.cpp	Uses `SetCatchTyp` when creating fault region blocks.
src/coreclr/jit/assertionprop.cpp	Uses `CatchTypIs` helper for EH-related filtering.

src/coreclr/jit/importer.cpp

src/coreclr/jit/block.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

src/coreclr/jit/importer.cpp

src/coreclr/jit/block.h

The BBCT_NONE macro was used in the bbCatchTyp field initializer at line 719 but not defined until line 1565, causing a compilation error. Move all BBCT_* defines and handlerGetsXcptnObj above the BasicBlock struct definition. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

src/coreclr/jit/assertionprop.cpp

Rename the abbreviated field and accessor names to use the full word: - bbCatchTyp -> bbCatchType - GetCatchTyp() -> GetCatchType() - SetCatchTyp() -> SetCatchType() - CatchTypIs() -> CatchTypeIs() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.

Copilot · 2026-02-28T14:27:49Z

src/coreclr/jit/importer.cpp

+    // A nullptr bbEntryState means that the block does not yet have a recorded pre-state,
+    // which corresponds to having no established (i.e. empty) stack depth on entry.


The new comment describing bbEntryState is misleading: nullptr does not mean "no recorded pre-state". The JIT intentionally uses nullptr to represent the common/empty entry state (stack depth 0), so the block can still have a recorded pre-state even when GetEntryState() returns null. Please adjust the wording to reflect that nullptr represents an established empty entry state (and cannot be used to distinguish "uninitialized" vs "empty").

Suggested change

// A nullptr bbEntryState means that the block does not yet have a recorded pre-state,

// which corresponds to having no established (i.e. empty) stack depth on entry.

// In general, the JIT uses a nullptr bbEntryState to represent the common/empty entry state

// (stack depth 0), i.e. an established empty pre-state rather than "no recorded pre-state".

Copilot · 2026-02-28T14:27:50Z

src/coreclr/jit/block.h

 // utilities that are polymorphic over basic block and scratch ranges
 // faster and simpler.
 //
+// Some non-zero value that will not collide with real tokens for bbCatchType


The comment above the BBCT_* constants says "Some non-zero value..." but BBCT_NONE is defined as 0. Consider rewording this to avoid the factual mismatch (e.g., describe these as reserved/special values, with BBCT_NONE being 0 and the others being non-token sentinel values).

Suggested change

// Some non-zero value that will not collide with real tokens for bbCatchType

// Special values for bbCatchType: BBCT_NONE is 0, the others are non-token sentinel values that do not

// collide with real exception handler class tokens.

AndyAyersMS and others added 9 commits February 26, 2026 07:53

JIT: Remove dead esRefcount comment in importer.cpp

83e416d

The esRefcount field was removed from EntryState long ago. Delete the stale commented-out reference to it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 26, 2026 18:26

github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 26, 2026

dotnet-policy-service bot assigned AndyAyersMS Feb 26, 2026

Copilot started reviewing on behalf of AndyAyersMS February 26, 2026 18:27 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

src/coreclr/jit/importer.cpp Outdated Show resolved Hide resolved

src/coreclr/jit/block.h Outdated Show resolved Hide resolved

Apply suggestions from code review

25b879b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 26, 2026 19:27

Copilot started reviewing on behalf of AndyAyersMS February 26, 2026 19:29 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

src/coreclr/jit/importer.cpp Outdated Show resolved Hide resolved

src/coreclr/jit/importer.cpp Outdated Show resolved Hide resolved

src/coreclr/jit/block.h Outdated Show resolved Hide resolved

AndyAyersMS and others added 2 commits February 26, 2026 11:38

format

2bb2321

Copilot AI review requested due to automatic review settings February 26, 2026 20:11

Copilot started reviewing on behalf of AndyAyersMS February 26, 2026 20:13 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

jakobbotsch reviewed Feb 26, 2026

View reviewed changes

src/coreclr/jit/assertionprop.cpp Outdated Show resolved Hide resolved

AndyAyersMS and others added 2 commits February 26, 2026 15:17

format

c5890dc

This was referenced Feb 27, 2026

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

[android-arm64] The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#6408

Open

This was referenced Feb 27, 2026

[FAIL] System.Net.Http.Functional.Tests.SocketsHttpHandler_HttpClientHandlerTest.LargeUriAndHeaders_Works #124487

Open

[android][clr] No peer certificates when executing System.Net.Http.Functional.Tests on Android emulator #124526

Open

Merge branch 'main' into CleanupBasicBlock

d367b71

Copilot AI review requested due to automatic review settings February 28, 2026 14:23

Copilot started reviewing on behalf of AndyAyersMS February 28, 2026 14:24 View session

Copilot AI reviewed Feb 28, 2026

View reviewed changes

build-analysis bot mentioned this pull request Feb 28, 2026

System.Transactions.Tests.OleTxTests.Recovery timeout #122621

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup basic block#124915

Cleanup basic block#124915
AndyAyersMS wants to merge 15 commits intodotnet:mainfrom
AndyAyersMS:CleanupBasicBlock

AndyAyersMS commented Feb 26, 2026

Uh oh!

dotnet-policy-service bot commented Feb 26, 2026

Uh oh!

AndyAyersMS commented Feb 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 28, 2026

Uh oh!

Copilot AI Feb 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// A nullptr bbEntryState means that the block does not yet have a recorded pre-state,
		// which corresponds to having no established (i.e. empty) stack depth on entry.

	// Some non-zero value that will not collide with real tokens for bbCatchType
	// Special values for bbCatchType: BBCT_NONE is 0, the others are non-token sentinel values that do not
	// collide with real exception handler class tokens.

Conversation

AndyAyersMS commented Feb 26, 2026

Uh oh!

dotnet-policy-service bot commented Feb 26, 2026

Uh oh!

AndyAyersMS commented Feb 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants