Skip to content
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

TLB Manager #420

Merged
merged 17 commits into from
Dec 26, 2024
Merged

TLB Manager #420

merged 17 commits into from
Dec 26, 2024

Conversation

broskoTT
Copy link
Contributor

@broskoTT broskoTT commented Dec 18, 2024

Issue

Related to #417

Description

Introduction into TLB manager class. This class should hold maps between tlb indexes and cores, and should hand out tlb_indicies. This might not be completelly true after this PR, which was more focused on getting things out from cluster.cpp file. But not too much stuff.

List of the changes

  • Created TLBManager class which lives inside TTDevice
  • Added get_tlb_manager to TTDevice
  • Moved dynamic_tlb_configs and dynamic_tlb_config_ordering maps.
  • Moved configure_tlb main logic with maps that hold these mapped tlbs
  • Moved all related functions from cluster.cpp
  • These moved maps/functions are still used throughout cluster.cpp, but this will be refactored further, so that the tlbmanager itself checks and hands out the writer.

Testing

Existing CI tests should cover this class.
Will try to add additional TLBManager focused tests.

API Changes

There are no API changes in this PR.

Copy link
Contributor

@pjanevskiTT pjanevskiTT left a comment

Choose a reason for hiding this comment

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

I like how the PR moves TLB logic from cluster downwards (Chip, TTDevice), but I think it is not in the right place in the layering. We can discuss this offline and get back to moving this if needed

device/api/umd/device/chip/local_chip.h Outdated Show resolved Hide resolved
device/chip/tlb_manager.cpp Outdated Show resolved Hide resolved
device/cluster.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@pjanevskiTT pjanevskiTT left a comment

Choose a reason for hiding this comment

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

just a few API comments, changes look great now

tests/api/test_tlb_manager.cpp Outdated Show resolved Hide resolved
tests/api/test_tlb_manager.cpp Outdated Show resolved Hide resolved
@broskoTT broskoTT changed the base branch from brosko/temp_base to main December 25, 2024 08:48
@broskoTT broskoTT merged commit 087bbe2 into main Dec 26, 2024
19 checks passed
@broskoTT broskoTT deleted the brosko/tlb_manager branch December 26, 2024 08:16
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.

3 participants