search

riscv-non-isa / riscv-trace-spec

RISC-V Processor Trace Specification
https://jira.riscv.org/browse/RVG-88
Creative Commons Attribution 4.0 International
165 stars 47 forks source link

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

Open IainCRobertson opened 2 weeks ago

IainCRobertson commented 2 weeks ago

@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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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