Merged
Conversation
Add iterator support to ArrayStack<T> enabling range-based for loops: - BottomUpOrder() iterates Bottom(0) to Top(0) using T* iterators - TopDownOrder() iterates Top(0) to Bottom(0) using ReverseIterator Convert 45 for-loop sites across 17 files from manual index-based Bottom(i)/BottomRef(i) iteration to use BottomUpOrder(). Co-authored-by: Copilot <[email protected]>
Convert top-to-bottom iteration loops across 6 files to use the new TopDownOrder() range-based for loop support on ArrayStack. Co-authored-by: Copilot <[email protected]>
In debug builds, ArrayStack now tracks a version counter that is incremented on Push, Emplace, Pop, and Reset. The BottomUpView and TopDownView capture this version on construction and assert it has not changed when destroyed, catching bugs where the stack is modified during range-based for iteration. Zero overhead in release builds via INDEBUG/DEBUGARG. Co-authored-by: Copilot <[email protected]>
github-actions
bot
added
the
area-CodeGen-coreclr
Member
Author
|
PTAL @dotnet/jit-contrib No diffs, tiny TP improvement. |
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds range-based iterator support to the JIT’s ArrayStack container and migrates a number of existing index-based loops to use the new iteration helpers, improving readability and reducing repeated indexing boilerplate across optimization/codegen phases.
Changes:
- Introduce
ArrayStack::BottomUpOrder()andArrayStack::TopDownOrder()iterable views (with DEBUG-time mutation detection). - Update many
ArrayStackiteration sites to use range-for over the new order helpers. - Minor local refactors in updated loops (variable naming/usage adjustments to match range-for).
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/jit/arraystack.h | Adds BottomUp/TopDown iterable views, reverse iterator, and DEBUG mutation versioning for iteration. |
| src/coreclr/jit/ssabuilder.cpp | Converts m_defs scans/prints to BottomUpOrder() range-for. |
| src/coreclr/jit/scev.cpp | Converts addOperands scans to BottomUpOrder() range-for. |
| src/coreclr/jit/rangecheckcloning.cpp | Converts bounds-check stack/location/group iteration to order views. |
| src/coreclr/jit/promotiondecomposition.cpp | Converts m_entries iteration to BottomUpOrder() range-for in multiple helpers. |
| src/coreclr/jit/promotion.cpp | Converts candidate store iteration to BottomUpOrder() range-for. |
| src/coreclr/jit/optimizer.cpp | Converts duplicated block/value-stack iteration to BottomUpOrder() range-for. |
| src/coreclr/jit/optcse.cpp | Converts choice/def-use iteration to TopDownOrder()/BottomUpOrder() range-for. |
| src/coreclr/jit/morph.cpp | Converts used-shared-temps iteration to TopDownOrder() range-for. |
| src/coreclr/jit/lower.cpp | Converts PUTARG_STK and param-reg-local mapping dumps to order views. |
| src/coreclr/jit/lclmorph.cpp | Converts assertion/store collections to BottomUpOrder() range-for. |
| src/coreclr/jit/inductionvariableopts.cpp | Converts multiple cursor/bound lists to BottomUpOrder() range-for. |
| src/coreclr/jit/flowgraph.cpp | Converts cycle-print stack iteration to TopDownOrder() range-for. |
| src/coreclr/jit/fgwasm.cpp | Converts SCC and interval iteration to order views. |
| src/coreclr/jit/fgopt.cpp | Converts predecessor/block list iteration to order views. |
| src/coreclr/jit/fgdiagnostic.cpp | Converts expected-node list printing to BottomUpOrder() range-for. |
| src/coreclr/jit/compiler.cpp | Converts param-reg-local mapping searches to BottomUpOrder() range-for. |
| src/coreclr/jit/codegencommon.cpp | Converts register graph node iteration to BottomUpOrder() range-for. |
| src/coreclr/jit/codegenarm.cpp | Converts param-reg-local mapping masking loop to BottomUpOrder() range-for. |
| src/coreclr/jit/assertionprop.cpp | Converts switch-block iteration to BottomUpOrder() range-for. |
Co-authored-by: Copilot <[email protected]>
Member
|
I like this, are there throughput measurements? EDIT: Sorry, didn't see you mentioned that TP is improved. Nice! |
kg
reviewed
Mar 2, 2026
kg
reviewed
Mar 2, 2026
kg
reviewed
Mar 2, 2026
kg
reviewed
Mar 2, 2026
kg
approved these changes
Mar 2, 2026
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.
Implement range-based iterator support for ArrayStack.