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

Add MSVC ARM64 support #1052

Merged
merged 1 commit into from
Feb 12, 2021
Merged

Add MSVC ARM64 support #1052

merged 1 commit into from
Feb 12, 2021

Conversation

@google-cla google-cla bot added the cla: yes label Oct 5, 2020
@janisozaur janisozaur mentioned this pull request Oct 5, 2020
10 tasks
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@janisozaur
Copy link
Contributor Author

Ping

@LebedevRI
Copy link
Collaborator

I would personally say that upstream CMake should land the fix first.

@janisozaur
Copy link
Contributor Author

That's a 6-year old issue and I haven't seen merge requests addressing it. I doubt fixing it upstream is on the horizon.

That said, cmake fix would be the best.

@LebedevRI
Copy link
Collaborator

Did anyone actually submit the patch upstream?
Either this is the right fix, and upstream will be okay with it,
or it's not and i'm not sure why wrong fix should be included into every downstream.

@janisozaur
Copy link
Contributor Author

@LebedevRI I'm trying to get Google benchmark packaged for arm64 with MSVC using vcpkg, but they, understandably, don't want new features, just the packaging fixes.

So far you only voiced concerns regarding packaging, but nothing regarding the timing code itself. Would you accept a PR with just the C++ bits extracted from this one and leave this one for packaging/cmake only?

Do you have any expectation of what should be the next steps? I can look for any patches to address this problem in cmake and assess when it might get released. I don't intend to fix the guy theyü⁷

problem

@LebedevRI
Copy link
Collaborator

See my previous comment - did anyone actually tried to upstream that patch into CMake proper?

@janisozaur janisozaur mentioned this pull request Feb 8, 2021
@janisozaur
Copy link
Contributor Author

@LebedevRI as your comments are with regards to CMake, I've split this patch to add reading counter support here and cmake stuff now lives at #1090

@LebedevRI
Copy link
Collaborator

I trust that you have manually ensured that the benchmarking results are actually meaningful there.

@LebedevRI LebedevRI merged commit ea5a5bb into google:master Feb 12, 2021
@janisozaur janisozaur deleted the msvc-arm64 branch July 4, 2021 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants