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

Printf mcs mutex #574

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Printf mcs mutex #574

wants to merge 6 commits into from

Conversation

mrutt92
Copy link
Contributor

@mrutt92 mrutt92 commented Sep 7, 2021

No description provided.

@taylor-bsg
Copy link
Contributor

is this one ready to go, max?

Comment on lines +34 to +35
extern int __bsg_pod_x; //The X Cord of the tile's pod (lives in DRAM)
extern int __bsg_pod_y; //The Y cord of the tile's pod (lives in DRAM)
Copy link
Contributor

Choose a reason for hiding this comment

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

You could grab these values from cfg_pod CSR at the beginning. There is no need to have the host set these.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess that's true. It save's a trivial few instruction words in the initialization to have the host set it. That CSR resets to the current pod but it can be set by the core to something else (like if it wants to read another pod's DRAM).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm honestly not 100% sure which is better. I think it's better to have the host set it? It's just a couple extra packets in the nbf and we don't need to entangle it with the semantics of the pod csr. But I'm not sure, I'm open to be talked down from that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it makes sense to keep these variable in DRAM, because if you change the pod x/y CSR then now you are reading those values from different pods' DRAM space. I think it's better off to make global x/y readable by CSR instructions (read-only), then it doesn't take up space in neither DRAM or DMEM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that makes a lot of sense. That'll also decouple these values from the semantics of the pod address register.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it necessary to access the X and Y coordinate of the pod? As a general principle we would like to avoid code that depends on this, since it is a virtualization hazard...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a global lock.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes why do we need a global lock across pods..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we don't? If each pod has its own IO node then there's no need.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment, there's only one IO node shared across pods. A global lock prevents the IO node from being slammed with packets from all pods.

@taylor-bsg
Copy link
Contributor

taylor-bsg commented Sep 29, 2021 via email

@taylor-bsg
Copy link
Contributor

taylor-bsg commented Sep 29, 2021 via email

@mrutt92
Copy link
Contributor Author

mrutt92 commented Sep 29, 2021

It prevents one IO node from being slammed and it also serves as keeping the messages from being garbled.

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