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

General overhaul of events and spec text #18

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

Conversation

bcstrongx
Copy link
Collaborator

No description provided.

@bcstrongx
Copy link
Collaborator Author

Perhaps unwisely, this PR now also includes major revisions to instruction counting events, nesting them all under INST.* events.

@bcstrongx
Copy link
Collaborator Author

Here's a pdf of the spec including this PR as it stands.
riscv-perf-events-latest.pdf

@bcstrongx bcstrongx changed the title Revise CACHE events to be hierarchical, counting cache accesses rather than instructions General overhaul of events and spec text Dec 16, 2024
@bcstrongx
Copy link
Collaborator Author

Here is the pdf including the latest changes in this PR
riscv-perf-events-latest.pdf

@bcstrongx
Copy link
Collaborator Author

Here's the latest and greatest draft spec incorporating this PR.
riscv-perf-events-latest.pdf

Copy link

@snehasish snehasish left a comment

Choose a reason for hiding this comment

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

Here are some comments based on the pdf dated 2025-01-22

  • In 2.1.1. I didn't see a way to distinguish conditional vs unconditional branches. I think this would be good to have for beq, bne etc.
  • Table 10 Inst Metrics would need to be updated for conditional branches too.
  • In 2.1.1. I'm not sure how much value inferrable vs uninferrable provides in addition to direct and indirect`. Is there something more implied here?
  • In 2.1.2 (page 8) -- "even non-speculative (.RET) DRSC events" --> DSRC?
  • In 2.1.2 (page 8) -- Table 6. ASRC Event Translation Sources -- s/standars/standards/
  • In 2.3 s/idenfies/indentifies/
  • In Table 13. CACHE Event Access or Operation Type -- I wonder if "outer cache" implies off-chip? If so we should just mention that instead.
  • Why shouldn't OPREF not increment if a cache is shared by multiple harts? Does this mean that we wouldn't count OPREF for an LLC?
  • In Table 22, s/oustanding/outstanding/
  • In Table 26, s/selcted/selected/ and s/standars/standards

Haven't looked at the TMA section yet..

@bcstrongx
Copy link
Collaborator Author

Here are some comments based on the pdf dated 2025-01-22

  • In 2.1.1. I didn't see a way to distinguish conditional vs unconditional branches. I think this would be good to have for beq, bne etc.

In RISC-V parlance, a "branch" is a conditional jump. So INST.BRANCH.RET counts retired conditional branches. But you're right that there's no single event to count all unconditional jumps. Ideally these would be implemented as "combination" events, where some event selector bits correspond to each transfer type, and combinations can be chosen. But I'd prefer not to require that, so we should probably specify any combinations that are interesting. I did that with TK and PRED, for instance. I can add unconditional.

Wonder if we want some combination of RETURN and CORSWAP (which is a return + call)? Or CALL and CORSWAP?

  • Table 10 Inst Metrics would need to be updated for conditional branches too.

Spoiler alert: I'm going to propose that we drop standardizing metrics, and stick to events. There might be an exception for topdown.

  • In 2.1.1. I'm not sure how much value inferrable vs uninferrable provides in addition to direct and indirect`. Is there something more implied here?

The trace spec uses inferrable/uninferrable. I think most are more accustomed to direct/indirect, but I included the other words to make sure everyone knows we're talking about the same thing.

  • In 2.1.2 (page 8) -- "even non-speculative (.RET) DRSC events" --> DSRC?
  • In 2.1.2 (page 8) -- Table 6. ASRC Event Translation Sources -- s/standars/standards/
  • In 2.3 s/idenfies/indentifies/

Thanks, will fix those.

  • In Table 13. CACHE Event Access or Operation Type -- I wonder if "outer cache" implies off-chip? If so we should just mention that instead.

I just meant a cache that can fill into this one. E.g., to the L2, the L3 is an outer cache, but the L1 is not. I made up the term though, maybe not the best one.

  • Why shouldn't OPREF not increment if a cache is shared by multiple harts? Does this mean that we wouldn't count OPREF for an LLC?

Since these are hart events, I'm assuming they should only be able to count events directly related to the hart. For a shared (external) LLC, I'd assume it would have its own PMU that could count accesses/misses/etc regardless of hart. So I assume that's where LLC prefetches would be counted, not in the hart.

  • In Table 22, s/oustanding/outstanding/
  • In Table 26, s/selcted/selected/ and s/standars/standards

Thanks.

Haven't looked at the TMA section yet..

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