[PATCH] D85031: [builtins] Unify the softfloat division implementation

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko created this revision.
atrosinenko added reviewers: koviankevin, joerg, efriedma, compnerd, scanon.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.
atrosinenko requested review of this revision.
atrosinenko updated this revision to Diff 282225.
atrosinenko added a comment.

Revert auto-linting


atrosinenko added a comment.

Here are the benchmark and fuzzing harness used to test this patch.

F12453076: fuzz_divXf3.sh <https://reviews.llvm.org/F12453076>

F12453075: divXf3_fuzzer.c <https://reviews.llvm.org/F12453075>

F12453074: bench_divXf3.sh <https://reviews.llvm.org/F12453074>

F12453073: divXf3_bench.c <https://reviews.llvm.org/F12453073>



================
Comment at: compiler-rt/lib/builtins/fp_div_impl.inc:248-249
+  if (quotient_UQ1 < (implicitBit << 1)) {
+    residualLo = (aSignificand << (significandBits + 1)) - quotient_UQ1 * bSignificand;
+    writtenExponent -= 1;
+
----------------
Interesting fact: swapping these two seemingly commuting lines makes code slower by 15-25%. This applies to current Clang as well as to `clang-8` from Ubuntu 20.04 repository.


This patch replaces three different pre-existing implementations of `div[sdt]f3` LibCalls with a generic one - like it is already done for many other LibCalls.

The patch was written with intent of the proof being as self-contained as possible, so future contributors do not have to re-prove it again. On the other hand, this makes it look clumsy, so feedback on **both** correctness and readability is highly appreciated.

When fuzzing with AFL++ (25M iterations each type width), just one error was found: for single precision, `0x1.fffffep-126F` divided by `2.F` was not correctly rounded to exactly `1.0`. On the other hand, this patch is just an intentionally simplified version of the full patch introducing a proper support for subnormal results - the full version fixes this issue as well.

This particular diff pretends to be an NFC refactoring and //technically// the above issue is not a regression because the original implementation yields the same result. :)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85031

Files:
  compiler-rt/lib/builtins/divdf3.c
  compiler-rt/lib/builtins/divsf3.c
  compiler-rt/lib/builtins/divtf3.c
  compiler-rt/lib/builtins/fp_div_impl.inc
  compiler-rt/lib/builtins/fp_lib.h
  compiler-rt/lib/builtins/int_util.h
  compiler-rt/test/builtins/Unit/divdf3_test.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85031.282225.patch
Type: text/x-patch
Size: 39137 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200731/0e5b11ea/attachment-0001.bin>