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

Avoid checking for arm/aarch64 in toolchain path #420

Merged

Conversation

georges-arm
Copy link
Contributor

Since the CMAKE_CXX_COMPILER contains the full path including directory components, this check will enable Arm intrinsics even on non-Arm platforms if the directory the compiler resides in happens to contain the substring "arm", for example:

    /home/parma/toolchains/g++-14
           ^^^

CMAKE_SYSTEM_PROCESSOR already specifies the target architecture so there is no need to additionally check the compiler string, therefore we simply remove this condition.

Since the CMAKE_CXX_COMPILER contains the full path including directory
components, this check will enable Arm intrinsics even on non-Arm
platforms if the directory the compiler resides in happens to contain
the substring "arm", for example:

        /home/parma/toolchains/g++-14
               ^^^

CMAKE_SYSTEM_PROCESSOR already specifies the target architecture so
there is no need to additionally check the compiler string, therefore we
simply remove this condition.
@adamjw24
Copy link
Member

LOL, good one.

Thanks for the fix, we'll have a look and probably merge soon.

@K-os
Copy link
Contributor

K-os commented Sep 25, 2024

Oh, thanks. Good catch.
I think we'll merge this for now, but internally I completely reworked the architecture detection heuristics. So with the next update on Github your fix will be overwritten.

Have you tried building VVdeC with your setup? Did that work? In VVdeC the heuristics are already as they will be in VVenC after the next update. Some feedback would be nice.

@K-os K-os self-requested a review September 25, 2024 10:10
@jbrdbg jbrdbg merged commit 06a223c into fraunhoferhhi:master Sep 25, 2024
8 checks passed
@georges-arm
Copy link
Contributor Author

Thanks for the quick review!

Have you tried building VVdeC with your setup? Did that work? In VVdeC the heuristics are already as they will be in VVenC after the next update. Some feedback would be nice.

I just had a quick try and VVdeC seems to work fine on both x86_64 and AArch64 as well as for cross compiling. 👍

@georges-arm georges-arm deleted the geoste01/no-check-toolchain-path branch September 25, 2024 15:15
@K-os
Copy link
Contributor

K-os commented Sep 25, 2024

thanks for the feedback

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.

4 participants