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

Introduce TARGET fallbacks in CMake #1752

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

xsacha
Copy link
Contributor

@xsacha xsacha commented Sep 6, 2018

X86(-64) targets for cross-compilation are likely to be for a simulator/testing and not a production environment. Regardless, we do not know information about the target yet, so choose a default of core2/penryn.

X86(-64) targets for cross-compilation are likely to be for a simulator/testing and not a production environment. Regardless, we do not know information about the target yet, so choose a default of core2/penryn.
@martin-frbg
Copy link
Collaborator

I am not sure if this premise is correct, and I expect the performance hit from using old kernels on new hardware would be much more noticable than on ARM. Perhaps just expanding the error message with a hint on exactly how to specify the TARGET to cmake would be better ?

@xsacha
Copy link
Contributor Author

xsacha commented Sep 6, 2018

Well that's why I originally left it empty because we cannot work out what hardware it will be targeting automatically.

@martin-frbg
Copy link
Collaborator

Maybe I am confused, but isn't the hunter ticket you linked to about cross-compiling from (whatever) to ARMV8 anyway ? And your emulator target scenario may be likely when the cross-compiler name suggests an Android NDK or the IOS equivalent coupled with x86 hardware (but I am not sure it would make sense to try to parse all that in the cmake file), but less so for general Linux/Windows cross builds.

@xsacha
Copy link
Contributor Author

xsacha commented Sep 6, 2018

No there is no issue on arm cause we can just target v8 or v7 and done.
It's when you cross compile to, for example, iOS x86-64 simulator.
I figure performance isn't the major concern for simulator but not sure.

@martin-frbg
Copy link
Collaborator

Alright, but how do we (or rather cmake) know someone wants to target IOS or Android simulator, rather than just cross-compile for a production environment with different OS ?

@xsacha
Copy link
Contributor Author

xsacha commented Sep 6, 2018

If it is iOS and x86, it has to be the simulator.
Android is a bit more tricky. Even so, they likely want to target all supported x86 android devices.

@brada4
Copy link
Contributor

brada4 commented Sep 8, 2018

X86 default is overly modern. While good for current 32bit windows (which you really dont simulate test much), modern 32bit linux would base at pentium or pentimpro (no MMX)

@martin-frbg
Copy link
Collaborator

As stated above I am not convinced we would want a fallback for x86 at all, where both the variety of targets and the loss of performance from choosing a lowest common denominator would be much greater than on other the other supported architectures.

@brada4
Copy link
Contributor

brada4 commented Sep 8, 2018

Indeed. Better to bail out early and get cpuid via bug report

@martin-frbg
Copy link
Collaborator

As I understand it this is about cross-compiling where there is no chance to autodetect the target platform, so it would be "tell them how to name their target to cmake" with no need for a bug report or cpuid.

@xsacha
Copy link
Contributor Author

xsacha commented Sep 9, 2018

How about if we detect iOS and x86_64, we use a sane default? They likely won't care about performance so much for simulator.
The simulator probably has a minimum spec itself.

@brada4
Copy link
Contributor

brada4 commented Sep 10, 2018

I disagree with statement about minimal specification in emulator. Look no further than qemu and intel SDE.
If you know the reasonable default for iOS please feel free to share.

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.

3 participants