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

fix: compilation for ubuntu22.04 arm64 #2259

Merged
merged 1 commit into from Nov 5, 2023

Conversation

jujimeizuo
Copy link
Contributor

error: On machines not built on x86-64 architecture, the cpuid.h file does not exist

/home/jujimeizuo/Workspace/openMVG/src/openMVG/system/cpu_instruction_set.hpp:18:12: fatal error: cpuid.h: No such file or directory
   18 |   #include <cpuid.h>
      |            ^~~~~~~~~
compilation terminated.

fixed: #1881 (comment), #if defined __aarch64__ check architecture

@pmoulon pmoulon changed the title fix: complie for ubuntu22.04 arm64 fix: compilation for ubuntu22.04 arm64 Oct 27, 2023
@pmoulon pmoulon added this to the 2.1 milestone Oct 27, 2023
#if defined __GNUC__
__cpuid_count(x, 0, out[0], out[1], out[2], out[3]);
return true;
#if not defined __aarch64__
Copy link
Member

@pmoulon pmoulon Oct 27, 2023

Choose a reason for hiding this comment

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

This one should not be needed, since you already return false for aarch64, just before, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, but the compiler thinks this is an error:

image

Copy link
Member

Choose a reason for hiding this comment

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

So we could have a unique if defined aarch64 return false here and remove the one on top, right?

Copy link
Member

Choose a reason for hiding this comment

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

@jujimeizuo could you update the code to have only one entry with __aarch64__ so it would ease later maintenance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep

@pmoulon pmoulon merged commit 085fbe4 into openMVG:develop Nov 5, 2023
9 checks passed
@pmoulon
Copy link
Member

pmoulon commented Nov 5, 2023

Thank you for your contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build fails on non-x86 systems
2 participants