JIT: Remove BBF_NEEDS_GCPOLL and BBF_HAS_SUPPRESSGC_CALL#129153
Closed
EgorBo wants to merge 1 commit into
Closed
Conversation
The GC poll insertion phase (fgInsertGCPolls) now relies solely on the method-global OMF_NEEDS_GCPOLLS flag to decide whether to run, and performs the full tree walk (blockNeedsGCPoll) in both tier0 (minopts) and tier1 (optimized) to find the blocks that need a poll. Previously tier0 relied on the per-block BBF_HAS_SUPPRESSGC_CALL / BBF_NEEDS_GCPOLL flags while only tier1 did the walk. The walk recognizes every case the per-block flags used to track: * SuppressGCTransition unmanaged calls (was BBF_HAS_SUPPRESSGC_CALL) * GT_GCPOLL nodes from Thread.FastPollGC (was BBF_NEEDS_GCPOLL) * slow tail calls dispatched via the JIT helper, x86-only, detected via IsTailCallViaJitHelper() (was BBF_NEEDS_GCPOLL) This drops the need to keep these per-block flags in sync across block splits/compaction. As a side effect it removes a redundant GC poll that BBF_NEEDS_GCPOLL could produce when it propagated (via BBF_SPLIT_GAINED / BBF_COMPACT_UPD) to both halves of a split FastPollGC block. Validated with SuperPMI asmdiffs on x64 (2.58M contexts) and x86: no regressions; the only diffs are the FastPollGC redundant-poll dedup in Array.Copy/CopyImpl/Queue.Grow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
area-CodeGen-coreclr
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the per-basic-block flags BBF_NEEDS_GCPOLL and BBF_HAS_SUPPRESSGC_CALL and updates GC-poll insertion (fgInsertGCPolls) to rely on the method-global OMF_NEEDS_GCPOLLS gate plus per-block tree walks to detect blocks requiring a GC poll.
Changes:
- Remove
BBF_NEEDS_GCPOLL/BBF_HAS_SUPPRESSGC_CALLand stop propagating them across block splits/compaction. - Update importer/morph to set only
OMF_NEEDS_GCPOLLSforThread.FastPollGC, SuppressGCTransition unmanaged calls, and tailcall-via-JIT-helper cases. - Make
fgInsertGCPollsscan trees in both minopts and optimized builds, and add detection for tail calls dispatched via the JIT helper.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/jit/morph.cpp | Stops setting per-block poll flags; sets OMF_NEEDS_GCPOLLS for relevant constructs and updates comments/logging. |
| src/coreclr/jit/importercalls.cpp | Removes BBF_NEEDS_GCPOLL marking for Thread.FastPollGC; relies on GT_GCPOLL + OMF_NEEDS_GCPOLLS. |
| src/coreclr/jit/flowgraph.cpp | Makes fgInsertGCPolls always use tree scanning; adds tailcall-via-helper detection and replaces per-block-flag checks. |
| src/coreclr/jit/block.h | Deletes the two basic-block flags and removes them from flag-propagation sets. |
| src/coreclr/jit/block.cpp | Removes debug display strings for the deleted flags. |
Comment on lines
+51
to
+55
| else if (call->IsTailCallViaJitHelper()) | ||
| { | ||
| // Slow tail calls dispatched via the JIT helper do not return, so the | ||
| // helper itself cannot poll for GC. We need to insert a poll before it. | ||
| blockMayNeedGCPoll = true; |
Comment on lines
+82
to
+98
| static bool blockHasTailCallViaHelper(BasicBlock* block) | ||
| { | ||
| for (Statement* const stmt : block->NonPhiStatements()) | ||
| { | ||
| if ((stmt->GetRootNode()->gtFlags & GTF_CALL) != 0) | ||
| { | ||
| for (GenTree* const tree : stmt->TreeList()) | ||
| { | ||
| if (tree->OperIs(GT_CALL) && tree->AsCall()->IsTailCallViaJitHelper()) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return false; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes the per-basic-block flags
BBF_NEEDS_GCPOLLandBBF_HAS_SUPPRESSGC_CALL. The GC-poll insertion phase (fgInsertGCPolls) now relies on the method-globalOMF_NEEDS_GCPOLLSflag to decide whether to run, and performs the full tree walk (blockNeedsGCPoll) in both tier0 (minopts) and tier1 (optimized). Previously tier0 relied on the per-block flags and only tier1 did the walk.The walk recognizes every case the per-block flags used to track:
BBF_HAS_SUPPRESSGC_CALL)GT_GCPOLLnodes fromThread.FastPollGC(wasBBF_NEEDS_GCPOLL)IsTailCallViaJitHelper()(wasBBF_NEEDS_GCPOLL)This drops the need to keep these per-block flags in sync across block splits/compaction. As a side effect it removes a redundant GC poll that
BBF_NEEDS_GCPOLLcould produce when it propagated (viaBBF_SPLIT_GAINED/BBF_COMPACT_UPD) to both halves of a splitFastPollGCblock.Validation
SuperPMI asmdiffs (Checked):
All diffs are the
FastPollGCredundant-poll dedup inArray.Copy/CopyImpl/Queue.Grow. Verified by disassembly that the essential poll after the no-GC bulk-move FCALL is preserved; only the extra poll (previously caused byBBF_NEEDS_GCPOLLpropagating across a block split) is removed.Note
This pull request (code and description) was generated with the assistance of GitHub Copilot.