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

ARM_ARCH_8A #94

Closed
a-w50 opened this issue Nov 18, 2022 · 4 comments
Closed

ARM_ARCH_8A #94

a-w50 opened this issue Nov 18, 2022 · 4 comments

Comments

@a-w50
Copy link

a-w50 commented Nov 18, 2022

Hi,

when I'm trying to cross-compile rapidyaml with an armv8 compiler, it does

#define __ARM_ARCH_8A__ 1 and not #define __ARM_ARCH_8__ 1.

The cpu.hpp in this library only checks for ARM_ARCH_8, which is not set in my case and hence it breaks. I'm not sure if this is just a typo here or if ARM_ARCH_8 really exists, but looking at openssl (https://github.com/openssl/openssl/blob/master/crypto/arm_arch.h) it also only checks for ARM_ARCH_8A.

@biojppm
Copy link
Owner

biojppm commented Nov 18, 2022

ATM I'm working with c4core on an armv8a 64bit platform and c4core definitely works there. I guess yours is 32bit?

Looking at what cpu.hpp does:

   #if defined(__aarch64__) || defined(_M_ARM64)
       #define C4_CPU_ARM64
       #define C4_CPU_ARMV8
       #define C4_WORDSIZE 8
   #else
       #define C4_CPU_ARM
       #define C4_WORDSIZE 4
       #if defined(__ARM_ARCH_8__) || (defined(__TARGET_ARCH_ARM) && __TARGET_ARCH_ARM >= 8)
           #define C4_CPU_ARMV8
//...

What happens if you add defined(__ARM_ARCH_8A__) to the 32 bit check? Does it fix your problem? Happy to accept a PR to address this.

@a-w50
Copy link
Author

a-w50 commented Nov 18, 2022

Right now, I'm add_compile_definitions(__ARM_ARCH_8__) to make it work again. Here is the output of my toolchains gcc

echo | ./crosstool/bin/armv8-rpi4-linux-gnueabihf-gcc -dM -E - | grep ARM
#define __ARM_SIZEOF_WCHAR_T 4
#define __ARM_FEATURE_SAT 1
#define __ARM_ARCH_ISA_ARM 1
#define __ARM_NEON 1
#define __ARMEL__ 1
#define __ARM_FP 14
#define __ARM_ARCH_8A__ 1
#define __ARM_NEON_FP 6
#define __ARM_FEATURE_IDIV 1
#define __ARM_SIZEOF_MINIMAL_ENUM 4
#define __ARM_PCS_VFP 1
#define __ARM_FEATURE_FMA 1
#define __ARM_FEATURE_LDREX 15
#define __ARM_FEATURE_UNALIGNED 1
#define __ARM_FEATURE_QBIT 1
#define __ARM_NEON__ 1
#define __ARM_ARCH_PROFILE 65
#define __ARM_32BIT_STATE 1
#define __ARM_FEATURE_CLZ 1
#define __ARM_ARCH_ISA_THUMB 2
#define __ARM_ARCH 8
#define __ARM_FEATURE_SIMD32 1
#define __ARM_FEATURE_CRC32 1
#define __ARM_FEATURE_DSP 1
#define __ARM_ARCH_EXT_IDIV__ 1
#define __ARM_EABI__ 1

Its a toolchain, build with crosstool-ng for raspberrypi4. What toolchain are you using and what is it's output of definitions?

Definitely adding defined(__ARM_ARCH_8A__) will solve the problem, I was just wondering if there is any compiler, setting __ARM_ARCH_8__

@biojppm
Copy link
Owner

biojppm commented Nov 18, 2022

On my side, I wrote this block of code many years ago and don't recall exactly why ARM_ARCH_8 is there.

But from a quick search, there is a mention to ARM_ARCH_8 in here, so it is not widely off the mark.

OTOH, on the same search I noticed this has only ARM64_ARCH_8. And no reference either in this.

Also, from the dump you provide, I notice the existing check (defined(__TARGET_ARCH_ARM) && __TARGET_ARCH_ARM >= 8) would work if it was actually (defined(__ARCH_ARM) && __ARCH_ARM >= 8).

So I'll submit a fix with both. I think it makes sense.

@a-w50
Copy link
Author

a-w50 commented Nov 18, 2022

Thanks! 💯

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

No branches or pull requests

2 participants