Simplify softfloat interface

[Timmmm]

Currently the softfloat interface relies on two global registers defined in Sail to return values, one of which is a mirror of fflags. Unfortunately this doesn't work if the model is compiled with -static or wrapped in an anonymous namespace (I am doing the latter in a similar way to @arichardson's method here).

This commit changes the design so that there is only a one-way dependency - the softfloat implementation does not depend on the model. This is done by returning the result of the floating point operation as a normal function return value, and then the model retrieves the new flags by simply calling another platform function (extern_float_flags()).

This is a bit cleaner, works with -static and also removes the need for write_fflags() and update_softfloat_fflags().

Tested with run_fp_tests.sh - all 82 tests pass.

Note, I have not updated the OCaml or Lem parts yet because I wanted to get agreement before doing all that work.

[ptomsich]

@Timmmm The clang-format pre-commit check is failing. Could you make sure that your PR follows the style (i.e., that you run clang-format)? Thx.

[Timmmm]

Ooops, fixed now.

[github-actions]

Unit Test Results

712 tests  ±0   396 ✔️  - 316   0s ⏱️ ±0s
    6 suites ±0       0 💤 ±    0 
    1 files   ±0   316 ❌ +316 

For more details on these failures, see this check.

Results for commit 979d6f2. ± Comparison against base commit 58cac61.

♻️ This comment has been updated with latest results.

[Timmmm]

The test failures are due to the OCaml emulator not building and are expected, as noted in the PR message.

[jrtc27]

I don't understand what's wrong with static linking, and wrapping random external code in an anonymous namespace is a "you pick up the pieces when it breaks" scenario, but using the return value directly is indeed cleaner if it works.

However, you seem to be doing three different things in the one commit, which makes it harder to review. Please separate out the return value change, the flags accrual change (to avoid the callback) and the flags getter change.

[Timmmm]

I don't understand what's wrong with static linking

Not static linking; static linkage. The Sail -static flag turns mach_bits zfloat_result; into static mach_bits zfloat_result;, so riscv_sail.h's extern mach_bits zfloat_result; no longer works.

and wrapping random external code in an anonymous namespace is a "you pick up the pieces when it breaks" scenario

This is me picking up the pieces! :-)

However, you seem to be doing three different things in the one commit, which makes it harder to review. Please separate out the return value change, the flags accrual change (to avoid the callback) and the flags getter change.

Fair point. It's actually a very small change but there are so many softfloat methods it's difficult to review like this. I've split it into two commits - the first has all the actual changes and updates the f16add/sub/div/mul methods. The second is just copy & pasting to all the other methods.

[jrtc27]

However, you seem to be doing three different things in the one commit, which makes it harder to review. Please separate out the return value change, the flags accrual change (to avoid the callback) and the flags getter change.

Fair point. It's actually a very small change but there are so many softfloat methods it's difficult to review like this. I've split it into two commits - the first has all the actual changes and updates the f16add/sub/div/mul methods. The second is just copy & pasting to all the other methods.

I appreciate the reasoning behind it, but having intermediate commits in the history that don't work is a no-no.

[Timmmm]

Ok I can squash it before merging. The three changes you mentioned are tightly linked and the changes to the code are quite minimal and easy to review. I would rather not spend a lot of time making intermediate commits that only fix 1/3 of the issue!

[Timmmm]

Actually the removal of write_fflags can probably be separated out fairly easily and it shouldn't require changes to OCaml. I'll make a separate PR for that.

[Timmmm]

Ok the first part is #290

[Timmmm]

Part 2 here: #340