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.

Sign up for GitHub

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

Jump to bottom

Add support for ZHAOXIN CPU #218

Merged
merged 10 commits into from
Feb 18, 2022
Merged

Add support for ZHAOXIN CPU #218

merged 10 commits into from
Feb 18, 2022

Conversation

AnvilaWang
Copy link
Contributor

Add support for CPU of vendor ZHAOXIN

toor1245
toor1245 reviewed Jan 21, 2022
View reviewed changes
gchatelet
gchatelet requested changes Jan 25, 2022
View reviewed changes
Copy link
Collaborator

@gchatelet gchatelet left a comment

Choose a reason for hiding this comment

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

Please address @toor1245 comments as well.

@AnvilaWang
Update cpuinfo_x86.h
d6f722d 
support for microarchitecture ZhangJiang, WuDaoKou, LuJiaZui, YongFeng. Change define name to VendorID string.
@AnvilaWang
Update impl_x86__base_implementation.inl
80a28e5 
Support for microarchitecture ZhangJiang, WuDaoKou, LuJiaZui and YongFeng. 
Change is_zx to is_zhaoxin.
Combine the logic in  GetX86CacheInfo.
Change Define name to VendorID string.
Mizux
Mizux reviewed Jan 27, 2022
View reviewed changes
@AnvilaWang
Update cpuinfo_x86.h
8b8c967 
Adjust format. add space before //.
@AnvilaWang
Copy link
Contributor Author

@gchatelet @toor1245 @Mizux
Are there any other modification requirements? Can I successfully submit the patch that supports ZHAOXIN CPUs?

@AnvilaWang
Update impl_x86__base_implementation.inl
d9c09f0 
Separate CPU_FEATURES_VENDOR_CENTAUR_HAULS and CPU_FEATURES_VENDOR_SHANGHAI
@toor1245
Copy link
Contributor

@gchatelet, @Mizux, please review PR

gchatelet
gchatelet reviewed Feb 16, 2022
View reviewed changes
Copy link
Collaborator

@gchatelet gchatelet left a comment

Choose a reason for hiding this comment

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

@AnvilaWang please fix the cosmetic change l.122. Then it's good to go.

@AnvilaWang
Copy link
Contributor Author

Hi, @gchatelet I don't think amd64 Windows /msvc (pull_request) error: " Configure: could not find any instance of Visual Studio." is my fault. Is it?

@Mizux
Copy link
Collaborator

Mizux commented Feb 17, 2022

Hi, @gchatelet I don't think amd64 Windows /msvc (pull_request) error: " Configure: could not find any instance of Visual Studio." is my fault. Is it?

No, my bad, we are using windows-latest github runner vm so they just migrate to VS 2022 so we should change our CMake Generator...
https://github.com/google/cpu_features/runs/5228975155?check_suite_focus=true#step:1:7

So you can safely ignore it, I'll create an issue to track and fix it...

@toor1245 toor1245 mentioned this pull request Feb 17, 2022
Merged
@toor1245
Copy link
Contributor

@AnvilaWang, added fix #223, could you rebase?

@gchatelet gchatelet merged commit 1d02169 into google:main Feb 18, 2022
@gchatelet gchatelet added this to the v0.7.0 milestone Mar 8, 2022
@gchatelet gchatelet added the enhancement New feature or request label Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mizux Mizux Mizux left review comments

@gchatelet gchatelet gchatelet approved these changes

@toor1245 toor1245 toor1245 approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
v0.7.0
Development

Successfully merging this pull request may close these issues.
4 participants
@AnvilaWang @toor1245 @Mizux @gchatelet