Details
- Reviewers
t.p.northover SjoerdMeijer dmgreen
Diff Detail
Unit Tests
Event Timeline
| clang/include/clang/Basic/arm_neon.td | ||
|---|---|---|
| 1213 | Clang should use the same feature guard as GCC here, as it is mandated by ACLE: __ARM_FEATURE_FRINT | |
| clang/include/clang/Basic/arm_neon.td | ||
|---|---|---|
| 1213 | Thanks for letting know!!! That was what I want to know. | |
| clang/lib/Basic/Targets/AArch64.cpp | ||
|---|---|---|
| 529–530 | I don't think this flag should exist as it's inconsistent with GCC. In GCC these instructions are enabled through -march=armv8.5-a . Generally, these intrinsics should use the same target as the assembly instructions? | |
| clang/lib/Basic/Targets/AArch64.cpp | ||
|---|---|---|
| 529–530 | You are right! I will update it. | |
| clang/lib/Basic/Targets/AArch64.cpp | ||
|---|---|---|
| 185 | I think we need to add a test new macro in: Preprocessor/aarch64-target-features.c | |
| clang/lib/Basic/Targets/AArch64.cpp | ||
|---|---|---|
| 185 | Sorry, wanted to say that we need to add a test for this new macro. | |
| clang/lib/Basic/Targets/AArch64.cpp | ||
|---|---|---|
| 185 | The macro is used in "arm_neon.h" which is generated at building time of clang as below so I thought we do not need to add the test for it. #if __ARM_ARCH >= 8 && defined(__aarch64__) && defined(__ARM_FEATURE_FRINT)
...
#ifdef __LITTLE_ENDIAN__
__ai float32x2_t vrnd32x_f32(float32x2_t __p0) {
float32x2_t __ret;
__ret = (float32x2_t) __builtin_neon_vrnd32x_v((int8x8_t)__p0, 9);
return __ret;
}
#else
__ai float32x2_t vrnd32x_f32(float32x2_t __p0) {
float32x2_t __rev0; __rev0 = __builtin_shufflevector(__p0, __p0, 1, 0);
float32x2_t __ret;
__ret = (float32x2_t) __builtin_neon_vrnd32x_v((int8x8_t)__rev0, 9);
__ret = __builtin_shufflevector(__ret, __ret, 1, 0);
return __ret;
}
#endif
...If it is not enough, I will add a test for it. | |
| clang/lib/Basic/Targets/AArch64.cpp | ||
|---|---|---|
| 185 | We let the preprocessor emit a new macro, so that's what we need to test, which I believe we need to do in that file I quoted earlier. The fact that it is used elsewhere is an indirect test. | |
| clang/lib/Basic/Targets/AArch64.cpp | ||
|---|---|---|
| 185 | Yep, I will add it. | |
| clang/test/Preprocessor/aarch64-target-features.c | ||
|---|---|---|
| 61 | would be good to have a negative test too, i.e. a CHECK-NOT of this for e.g. v8.4a. | |
| clang/test/Preprocessor/aarch64-target-features.c | ||
|---|---|---|
| 61 | Yep, I will add it. | |
Sorry, one more question. This implements the vector variants. How about the scalar ones? Have they been implemented already?
um... I have not checked the scalar intrinsics for fp rounding. I have checked 'vrnd32/64x' which are neon vector intrinsics for v8.5-a. If the scalar one is not implemented yet, it could be other commit. Let me have a look.
Oops, I missed to add Differential Revision. It is merged with 9b302513f6d82f0ca989b3bb1f5ffc592ed866b7.
Clang should use the same feature guard as GCC here, as it is mandated by ACLE: __ARM_FEATURE_FRINT
See the relevant section in https://developer.arm.com/documentation/101028/0012/8--Data-processing-intrinsics