[libc] Fix feature check for riscv #145169
Conversation
Signed-off-by: Mikhail R. Gadelha <[email protected]>
|
@llvm/pr-subscribers-libc Author: Mikhail R. Gadelha (mikhailramalho) ChangesThis PR fixes the feature detection for RISC-V floating-point support in LLVM's libc implementation. The For half-precision support, the implementation checks for the Zfhmin extension as RVA22 and RVA23 profiles only require Zfhmin rather than the full Zfh extension. Zfh also implies Zfhmin, so checking for Zfhmin should cover all cases. Full diff: https://github.com/llvm/llvm-project/pull/145169.diff 1 Files Affected:
diff --git a/libc/src/__support/macros/properties/cpu_features.h b/libc/src/__support/macros/properties/cpu_features.h
index 3677e1fc3275c..457a2b7869d40 100644
--- a/libc/src/__support/macros/properties/cpu_features.h
+++ b/libc/src/__support/macros/properties/cpu_features.h
@@ -61,15 +61,15 @@
#if defined(__riscv_flen)
// https://github.com/riscv-non-isa/riscv-c-api-doc/blob/main/src/c-api.adoc
-#if (__riscv_flen & 0x10)
+#if (__riscv_arch_test && __riscv_zfhmin)
#define LIBC_TARGET_CPU_HAS_RISCV_FPU_HALF
#define LIBC_TARGET_CPU_HAS_FPU_HALF
#endif // LIBC_TARGET_CPU_HAS_RISCV_FPU_HALF
-#if (__riscv_flen & 0x20)
+#if (__riscv_flen >= 32)
#define LIBC_TARGET_CPU_HAS_RISCV_FPU_FLOAT
#define LIBC_TARGET_CPU_HAS_FPU_FLOAT
#endif // LIBC_TARGET_CPU_HAS_RISCV_FPU_FLOAT
-#if (__riscv_flen & 0x40)
+#if (__riscv_flen >= 64)
#define LIBC_TARGET_CPU_HAS_RISCV_FPU_DOUBLE
#define LIBC_TARGET_CPU_HAS_FPU_DOUBLE
#endif // LIBC_TARGET_CPU_HAS_RISCV_FPU_DOUBLE
|
| #if defined(__riscv_flen) | ||
| // https://github.com/riscv-non-isa/riscv-c-api-doc/blob/main/src/c-api.adoc | ||
| #if (__riscv_flen & 0x10) | ||
| #if (__riscv_arch_test && __riscv_zfhmin) |
There was a problem hiding this comment.
We hit a compile error after this:
../../third_party/llvm-libc/src/src/__support/macros/properties/cpu_features.h:64:27: error: '__riscv_zfhmin' is not defined, evaluates to 0 [-Werror,-Wundef]
64 | #if (__riscv_arch_test && __riscv_zfhmin)
| ^
Should it be #if ... defined(__riscv_zfhmin) instead?
There was a problem hiding this comment.
I guess #if defined(__riscv_zfhmin) should work too, does it fix the issue for you?
There was a problem hiding this comment.
I don't have my local build configured for risc-v to confirm, but yes I'm pretty sure that would fix it.
There was a problem hiding this comment.
Do you want me to open a PR, or do you prefer to do it yourself?
There was a problem hiding this comment.
I don't have much risc-v or llvm-libc experience, so it's probably better if you do it.
This is a follow-up to llvm#145169, which would break compiles when __riscv_zfhmin is not defined.
This is a follow-up to #145169, which would break compiles when __riscv_zfhmin is not defined.
This is a follow-up to llvm#145169, which would break compiles when __riscv_zfhmin is not defined.
5 participants
This PR fixes the feature detection for RISC-V floating-point support in LLVM's libc implementation.
The
__riscv_flenmacro represents the floating-point register width in bits (32, 64, or 128). Since Extension D is specifically documented as implying F, we can use simple >= comparisons to detect them.For half-precision support, the implementation checks for the Zfhmin extension as RVA22 and RVA23 profiles only require Zfhmin rather than the full Zfh extension. Zfh also implies Zfhmin, so checking for Zfhmin should cover all cases.