openlibm icon indicating copy to clipboard operation
openlibm copied to clipboard

riscv: Fix feholdexcept()

Open AlekseyZhmulin opened this issue 6 months ago • 5 comments

The feholdexcept() function must store the current floating point environment in *__envp. Related to #321

AlekseyZhmulin avatar May 07 '25 09:05 AlekseyZhmulin

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 72.09%. Comparing base (c9c6fd6) to head (361868e).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #324   +/-   ##
=======================================
  Coverage   72.09%   72.09%           
=======================================
  Files         233      233           
  Lines        6139     6139           
  Branches     1607     1607           
=======================================
  Hits         4426     4426           
  Misses       1420     1420           
  Partials      293      293           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar May 07 '25 09:05 codecov[bot]

The problem is that feholdexcept() does not save floating point environment, but feupdateenv() loads floating point environment. Therefore, in the lrint() function, "fenv_t env" is not initialized and a random value from the stack is loaded into the fcsr register. https://github.com/JuliaMath/openlibm/blob/master/src/s_lrint.c#L59. This throws an "Illegal instruction" exception.

AlekseyZhmulin avatar May 09 '25 14:05 AlekseyZhmulin

Musl: https://git.musl-libc.org/cgit/musl/tree/src/fenv/feholdexcept.c?id=6f2e5607d2d463dfc342489660e089553b749dcf

ViralBShah avatar May 21 '25 15:05 ViralBShah

@inkydragon Thoughts on whether we should merge?

ViralBShah avatar Jun 02 '25 21:06 ViralBShah

I also added clearing of all exception flags in feholdexcept()

AlekseyZhmulin avatar Jun 13 '25 18:06 AlekseyZhmulin