Skip to content

feat(rocjitsu) Enable const iteration for IntrusiveList - #14420

Merged
atgutier merged 1 commit into
developfrom
users/agutierr/intrusive-list-const-iteration
Oct 28, 2026
Merged

atgutier merged 1 commit into
developfrom
users/agutierr/intrusive-list-const-iteration

Conversation

@atgutier

Copy link
Copy Markdown
Contributor

Const instruction lists cannot be traversed because IntrusiveList has only mutable begin/end methods. Its ConstIterator alias also names a different intrusive node type when T becomes const.

Base both iterators on the same node, preserve element constness, and add const begin/end plus cbegin/cend. Keep mutable iterator references writable, allow conversion to the const iterator, and expose an unqualified value_type. Register a unit test for empty, forward, and reverse const traversal and iterator traits.

Verified: Release builds of rocjitsu_tests and hsa_hooks_unit_test; CTest: 1 intrusive-list test, 31 code/instrumentor tests, and 57 HSA hook unit tests passed.

Closes #14023

Copilot AI balanced review requested due to automatic review settings October 27, 2026 21:58
@atgutier
atgutier requested a review from a team as a code owner October 27, 2026 21:58
@therock-pr-bot

therock-pr-bot Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues block the IntrusiveList const-traversal changes.

No open issues 🎉

What changed in this PR

Enables const traversal of rocjitsu’s IntrusiveList, allowing callers to iterate instruction lists through const BasicBlock accessors and addressing #14023.

Changes:

  • Uses the same underlying node type for mutable and const iterators, preserves reference constness, and supports mutable-to-const conversion.
  • Adds const begin()/end() and cbegin()/cend() accessors.
  • Registers a unit test covering empty lists, forward and reverse const traversal, and iterator traits.
File Description
emulation/​rocjitsu/​tests/​util_intrusive_list_test.cpp Tests const traversal, conversion, and iterator traits.
emulation/​rocjitsu/​tests/​CMakeLists.txt Adds the intrusive-list test to the test executable.
emulation/​rocjitsu/​lib/​util/​include/​util/​intrusive_list.h Corrects iterator types and adds const traversal accessors.

Review effort: Balanced

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Const instruction lists cannot be traversed because IntrusiveList has
only mutable begin/end methods. Its ConstIterator alias also names a
different intrusive node type when T becomes const.

Base both iterators on the same node, preserve element constness, and add
const begin/end plus cbegin/cend. Make mutable iterators yield `T &`
instead of `const T &`, allow conversion to the const iterator, and
expose an unqualified value_type. Remove the unused ConstPointer,
ConstReference, and ConstNodeReference aliases. Register a unit test
for empty, forward, and reverse const traversal and iterator traits.

Verified: Release builds of rocjitsu_tests and hsa_hooks_unit_test;
CTest: 1 intrusive-list test, 31 code/instrumentor tests, and 57 HSA
hook unit tests passed.

Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>
@atgutier
atgutier force-pushed the users/agutierr/intrusive-list-const-iteration branch 2 times, most recently from 775b577 to dd25c3b Compare October 28, 2026 16:00
@atgutier
atgutier merged commit 0d3cab4 into develop Oct 28, 2026
39 checks passed
@atgutier
atgutier deleted the users/agutierr/intrusive-list-const-iteration branch October 28, 2026 16:36
@github-actions

Copy link
Copy Markdown
Contributor

rocjitsu post-commit fuzz passed

The 32-worker multiplexer campaigns for commit 0d3cab4b1568 completed without recording a crash or hang.

  • ASan+UBSan (6-minute): 571,324 input executions (1,618.06 inputs/s)
  • TSan (6-minute): 515,559 input executions (1,476.25 inputs/s)

View the workflow run.

@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[rocjitsu][util] Const iteration for util::IntrusiveList

3 participants