search

shibatch / sleef

SIMD Library for Evaluating Elementary Functions, vectorized libm and DFT
https://sleef.org
Boost Software License 1.0
666 stars 133 forks source link

Risc V and gcc-13 #579

Closed atupone closed 4 days ago

atupone commented 1 month ago

See https://bugs.gentoo.org/939400

It seems that sleef (3.6.1), on Risc-V, check for SIMD instruction that belongs to the v0.11.x draft but then during build uses instructions in the v1.0.

gcc-13 only support the v0.11.x draft

I suggest checking instruction set belonging to the v1.0 (e.g. vfloat64m1x4_t)

Unfortunately I don't have a risc-v hardware to test

shibatch commented 1 month ago

As for the RISC-V architecture, gcc-13 is not supported, see README. I get so many errors when building with gcc-13. It is unlikely that gcc-13 will ever be supported. I tried with gcc-14.2.0 and it passed the tests.

orlitzky commented 1 month ago

Apparently it does build OK and pass tests once RVV is manually disabled -- we discovered this on the Gentoo bug.

I think all of the build failures are due to RVV being misdetected. With gcc-13, both RVV-1.0 and RVV-2.0 are detected but neither are implemented. This leads to build failures because sleef actually needs v1.0 to be fully implemented. The mis-detection with gcc-13 is due to the CMake tests that look for some types present in the v0.11.x draft of RVV. These are present in gcc-13 (so the test passes), but the v1.0 types aren't (so the build fails).

As @atupone suggests this can probably be fixed by checking for some types that are present in v1.0 and v2.0 only. I would be happy to test. I already have gcc-14 installed and it would be easy to install gcc-13 as well.

blapie commented 1 month ago

@luhenry @GlassOfWhiskey @ericlove I thought I should draw your attention on this issue. Can you think of a way to make feature detection a bit more robust for RVV and avoid the mis-detection mentioned above for gcc 13+?

I will see what we can do regarding testing with gcc 13/14 in CI.

blapie commented 1 month ago

@atupone @orlitzky Thanks for reporting the issue, feel free to suggest a fix. We will be able to test it locally with the appropriate compilers, although we also don't have access to hardware.

ericlove commented 1 month ago

@luhenry @GlassOfWhiskey @ericlove I thought I should draw your attention on this issue. Can you think of a way to make feature detection a bit more robust for RVV and avoid the mis-detection mentioned above for gcc 13+?

The standard way of testing for RVV v1.0 ISA compatibility would be to compare the __riscv_v macro against 1000000, but in this case I think we should also compare the intrinsics API version against 12000 to ensure the tuple types are defined.

blapie commented 2 weeks ago

Hi! Is someone able to give that a go? If I understand the suggestion is to use a different intrinsic/type in the detection mechanism, e.g. https://github.com/shibatch/sleef/blob/master/Configure.cmake#L664

Currently the code that's used for checking is

    vint32m2_t r = __riscv_vmv_v_x_i32m2(1, 2 * __riscv_vlenb() * 8 / 32); }"

I'm not familiar with RISC-V types and intrinsics, let alone across version of the architecture, what would be a good choice of type or intrisinc to avoid this misdetection?

orlitzky commented 2 weeks ago

I guess I will take a stab at it using @ericlove's suggestion

blapie commented 2 weeks ago

Thank you @orlitzky!

ericlove commented 2 weeks ago

Indeed, sorry to have dropped the ball on this, but thanks @orlitzky for taking it on, and I'd be happy to review. 👍

orlitzky commented 2 weeks ago

An RVV1 attempt here: https://github.com/shibatch/sleef/pull/602

I have two questions now:

  1. I was able to find the spec for __riscv_v_intrinsic, but I couldn't find any documentation for __riscv_v. Both GCC and clang seem to know about it, but I'd be happier if I saw it written down somewhere. Anyone know?
  2. What should we do about RVV2? The default build still fails with GCC 13 because RVV2 support is detected. (I don't even know what RVV2 is referring to exactly.)
ericlove commented 1 week ago
  • I was able to find the spec for __riscv_v_intrinsic, but I couldn't find any documentation for __riscv_v. Both GCC and clang seem to know about it, but I'd be happier if I saw it written down somewhere. Anyone know?
  • What should we do about RVV2? The default build still fails with GCC 13 because RVV2 support is detected. (I don't even know what RVV2 is referring to exactly.)

@orlitzky thanks for making https://github.com/shibatch/sleef/pull/602 ! Regarding these questions:

  1. The __riscv_v macro is not part of the intrinsics specification, but is instead generated by the presence of the "V" extension in the ISA string of the compiler target. This is documented in the "Architecture Extension Test Macros" section of the RISC-V C API doc.
  2. The RVVM1 and RVVM2 targets refer to two different versions of the SLEEF RVV port, corresponding to the value of LMUL in each. To handle RVVM2, you just need to make the same change as for RVVM1 in the RVVM2 tests defined a few lines later.

Thanks again, and happy to approve #602 when the latter change is made.

orlitzky commented 1 week ago

Thanks! Everything is becoming clearer now. I hadn't made the connection between the M2 in RVVM2 and and the "m2" in the function name.

I've updated the PR. The two tests for RVVM1 and RVVM2 are identical now, which is begging for a follow-up PR to consolidate them.