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

Clarify which addresses are stored in the JTC - Issue 169 #171

Open
wants to merge 2 commits into
base: updates-2.0.4
Choose a base branch
from

Conversation

IainCRobertson
Copy link
Collaborator

@pdonahue-ventana I have used a slightly different wording from your suggestion but I think this is even more explicit. Are you okay with this?

@pdonahue-ventana
Copy link
Contributor

This talks about the contents of the cache but not how the lookup is done. I wasn't concerned about ambiguity in the contents of the cache but rather in how you determine which cache index to look in.

You could have used the PC of the jump itself. This would be how a predictor would work in the frontend of a pipeline (since it doesn't yet know the target and therefore can't use the target's PC). That may be what some people are more familiar with. However, I believe that the intent is that you use the PC of the target. This allows multiple jumps to have the same target and all of them will hit after the first jump happens (unlike using the jump PC which would need to warm up the cache for each jump). It also allows one jump to have multiple targets without thrashing the cache.

@IainCRobertson
Copy link
Collaborator Author

Paul we agree on the behaviour but I don't see how my latest change isn't sufficient or clear. I've now stated explicitly that the address in the cache is the target, and it is also explicit that the index is formed from the LSBs of this address. So why would anyone think that you would look up in the cache using the jump and not the target? As you already pointed out, there is only one 'it' in the final sentence.

However, I've made a further update so it really is belt and braces!

@pdonahue-ventana
Copy link
Contributor

You previously added "The addresses stored in the cache are the targets of uninferable jumps." That talks about the contents of the cache but not the indexing. Now you've added "the entry in the cache at the index derived from the jump target address" which explicitly says that the index is derived from the target address rather than the address of the jump. That's what I was looking for.

@IainCRobertson
Copy link
Collaborator Author

@ved-rivos this is now with you for final approval

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