Skip to content

Make attrmap able to alter memory attributes as well #5162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mmicko
Copy link
Member

@mmicko mmicko commented Jun 4, 2025

It was not possible to alter memory attributes with attrmap.

By using selected_members iterator instead we are covering all as we already do plus memories.

attrmap_apply was removed from processes block, but also used a chance to cleanup and use selected_processes to handle rest of changes needed

@KrystalDelusion KrystalDelusion self-requested a review June 5, 2025 16:19
@KrystalDelusion
Copy link
Member

Since we are touching this file we should also review the deprecation warning on it. From #4768:

Note:
Design::selected_whole_modules() is marked deprecated because I'm not convinced attrmap should be ignoring boxes, even though that is the current behaviour.

Design::selected_whole_modules() previously iterated over all wholly selected modules while silently ignoring any boxed modules. Most of the previous uses of this method were updated in #4768 to use the equivalent, Design::selected_unboxed_whole_modules(), but attrmap wasn't because I wasn't sure how it should handle boxes.

For comparison, setattr uses Design::all_selected_modules() which includes boxes. wbflip specifically says "Blackbox cells are not effected by this command" and uses Design::selected_modules(RTLIL::SELECT_ALL, RTLIL::SB_EXCL_BB_ONLY). blackbox uses Design::selected_whole_modules_warn(true) to include whiteboxes but warns on blackboxes and partially selected modules.

For the non -modattr mode it is using the default Design::selected_modules() which includes partially selected modules, and warns on any boxes in the selection. If we end up including boxes for -modattr we may also want to change the other one to match.

Rename `selected_members` iterator to memb.
Add comment on `selected_processes` loop for clarity.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants