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

Update PerfCounters.cc #3771

Closed
wants to merge 1 commit into from
Closed

Update PerfCounters.cc #3771

wants to merge 1 commit into from

Conversation

20urc3
Copy link

@20urc3 20urc3 commented Jun 26, 2024

Fix bug #3770

@@ -100,6 +100,7 @@ enum CpuMicroarch {
AMDZen2,
AMDZen3,
AMDZen4,
AMDRyzen9,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't need to add this; this CPU is Zen 4.

Copy link
Author

@20urc3 20urc3 Jun 26, 2024

Choose a reason for hiding this comment

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

Try it. Not supported. I think we need this line to add the following { AMDRyzen9, "AMD Ryzen 9", 0x5100d1, 0, 0x6F, 10000, PMU_TICKS_RCB | PMU_SKID_UNBOUNDED },

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you put 0x6f there? That's the llsc_cntr_event and it makes no sense to set that for an x86 CPU.

Using Zen4 should work. If it doesn't, something else is quite broken.

Copy link
Author

Choose a reason for hiding this comment

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

I've just followed the pattern I saw before. After compiling this version it works on my end, so maybe the issue is not fix by my did but was already fixed between the latest package release and the git version?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, this change doesn't actually do anything because nothing detects this new architecture type AMDRyzen9..

@@ -100,6 +100,7 @@ enum CpuMicroarch {
AMDZen2,
AMDZen3,
AMDZen4,
AMDRyzen9,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you put 0x6f there? That's the llsc_cntr_event and it makes no sense to set that for an x86 CPU.

Using Zen4 should work. If it doesn't, something else is quite broken.

@rocallahan rocallahan closed this Jun 26, 2024
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