Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
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 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
inkydragon
left a comment
There was a problem hiding this comment.
We're doing the same thing as BSD.
I'm not sure this works.
https://github.com/freebsd/freebsd-src/blob/master/lib/msun/riscv/fenv.h#L188-L195
|
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. |
|
@inkydragon Thoughts on whether we should merge? |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the feholdexcept() function to correctly store the current floating point environment in *__envp and update its return value to indicate success.
- Added the __rfs(*__envp) call to store the floating point state.
- Changed the return value from -1 to 0 to align with a successful operation.
Comments suppressed due to low confidence (2)
include/openlibm_fenv_riscv.h:193
- Consider adding a brief inline comment explaining the purpose of __rfs in storing the floating point environment for better maintainability.
__rfs(*__envp);
include/openlibm_fenv_riscv.h:197
- Returning 0 indicates success; please confirm that this change fully aligns with the API contract for feholdexcept() in this context.
return (0);
|
I also added clearing of all exception flags in feholdexcept() |
The feholdexcept() function must store the current floating point environment in *__envp.
Related to #321