This patch fixes two problems #592
Conversation
* On aarch32, cbrt(4) was returned instead of M_PI/2. * Due to a bug in gcc-13, sign of atanf(+-0) was wrong. This patch also adds testing for the first problem. The second problem is detected by the existing tester.
| @@ -908,6 +911,9 @@ EXPORT CONST VECTOR_CC vfloat xtanf(vfloat d) { | |||
|
|
|||
| q = vrint_vi2_vf(vmul_vf_vf_vf(d, vcast_vf_f((float)(2 * M_1_PI)))); | |||
| u = vcast_vf_vi2(q); | |||
| #if (defined(ENABLE_PUREC_SCALAR) || defined(ENABLE_PURECFMA_SCALAR) || defined(ENABLE_VXE) || defined(ENABLE_VXENOFMA) || defined(ENABLE_VXE2) || defined(ENABLE_VXE2NOFMA)) && !defined(__clang__) && __GNUC__ == 13 | |||
There was a problem hiding this comment.
Is it just a problem with gcc 13?
If so we should seek to understand what is going on, because that might have repercussions elsewhere that we haven't noticed.
There was a problem hiding this comment.
If you wish to merge I would recommend creating an issue, so we don't forget about it.
There was a problem hiding this comment.
There seems no problem with gcc-12 and gcc-14.
Of course it is better to understand what is going on, but that takes quite a bit of time.
I will create an issue for this matter.
| @@ -1770,7 +1776,7 @@ EXPORT CONST VECTOR_CC vfloat xatanf(vfloat d) { | |||
| t = vreinterpret_vf_vm(vxor_vm_vm_vm(vand_vm_vo32_vm(veq_vo_vi2_vi2(vand_vi2_vi2_vi2(q, vcast_vi2_i(2)), vcast_vi2_i(2)), vreinterpret_vm_vf(vcast_vf_f(-0.0f))), vreinterpret_vm_vf(t))); | |||
|
|
|||
| #if defined(ENABLE_NEON32) || defined(ENABLE_NEON32VFPV4) | |||
| t = vsel_vf_vo_vf_vf(visinf_vo_vf(d), vmulsign_vf_vf_vf(vcast_vf_f(1.5874010519681994747517056f), d), t); | |||
| t = vsel_vf_vo_vf_vf(visinf_vo_vf(d), vmulsign_vf_vf_vf(vcast_vf_f(1.570796326794896557998982), d), t); | |||
There was a problem hiding this comment.
Just a comment that in general I would recommend using hexfloat representation for constants, that would spare a conversion from double to float, or even from the literal to floating point.
Not asking to change now but that is smth we fix library-wide. That will also avoid carrying some many decimals, which is prone to errors.
Defining constants via macros could also prevent this type of errors.
There was a problem hiding this comment.
I agree that hexadecimal is better, but I was wondering which is better this time because of the MSVC case.
I will use hexadecimal next time.
Checklist
What is the purpose of this pull request?
What changes did you make?
Does this PR relate to any existing issue?
These changes are required to implement #574, because these problems are also detected by the new tester.