fix: detect loongarch cpu in header #2128
Conversation
Based on [GCC Loongarch Config](https://github.com/gcc-mirror/gcc/blob/master/gcc/config/loongarch/loongarch-c.cc)
|
@briansmith can I ping you to review this pr? |
| @@ -42,9 +42,9 @@ | |||
| #elif !(defined(__ORDER_LITTLE_ENDIAN__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)) && \ | |||
| !(defined(__ORDER_BIG_ENDIAN__) && (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)) | |||
| #error "Unsupported endianness" | |||
| #elif defined(__LP64__) | |||
| #elif defined(__LP64__) || defined(__loongarch64) || defined(__loongarch_lp64) | |||
There was a problem hiding this comment.
Why is this needed? Does GCC for the 64-bit target not define __LP64__? https://loongson.github.io/LoongArch-Documentation/LoongArch-toolchain-conventions-EN.html says it is defined so this shouldn't be needed.
| #define OPENSSL_64_BIT | ||
| #elif defined(__ILP32__) | ||
| #elif defined(__ILP32__) || defined(__loongarch__) |
There was a problem hiding this comment.
Wouldn't this addition be better placed ...
There was a problem hiding this comment.
__loongarch__ is defined for 64-bit systems as well, so this expression doesn't seem correct.
There was a problem hiding this comment.
__loongarch__is defined for 64-bit systems as well, so this expression doesn't seem correct.
Yes, but __LP64__ will be set for 64-bit systems, right? Otherwise the logic for the other 32-bit archs wouldn't work either. Keep in mind that the thing we're really working around is that these old versions of GCC didn't set __ILP32__ for many 32-bit architectures.
| #define OPENSSL_64_BIT | ||
| #elif defined(__ILP32__) | ||
| #elif defined(__ILP32__) || defined(__loongarch__) | ||
| #define OPENSSL_32_BIT | ||
| // Versions of GCC before 10.0 didn't define `__ILP32__` for all 32-bit targets. | ||
| #elif defined(__MIPSEL__) || defined(__MIPSEB__) || defined(__PPC__) || defined(__powerpc__) || defined(__csky__) || defined(__XTENSA__) |
There was a problem hiding this comment.
...here, with all the other similar ones?
|
Any updates on this? Would love to get this resolved for the next release. |
|
Is this patch intended to ensure compatibility with some outdated compilers that haven't been upstreamed yet? I believe the upstream version (compatible with LoongArch ABI 2.0) doesn't require it. |
Based on GCC Loongarch Config