This is an archive of the discontinued LLVM Phabricator instance.

[builtins] Add more test cases for __div[sdt]f3 LibCalls
ClosedPublic

Authored by atrosinenko on Jul 30 2020, 4:33 AM.

Details

Summary

The existing tests for softfloat division for single and double precisions contain only two test cases each and only the quad precision test has somewhat more test cases.

This patch

  • makes the three tests look more uniformly
  • specifies types of integer and fp literals more stricter
  • adds more test cases (basically, everything that was in some of test files is put everythere)
    • tests are added for obviously special cases of +/-Inf, +/-0 and some more implementation-specific cases such as divisor being almost 1.0
  • makes NaN in the second test case of divtf3 to be sNaN instead of testing for qNaN again

This patch is sent in preparation for unifying the division implementations and implementing subnormal result handling.

Diff Detail

Event Timeline

atrosinenko created this revision.Jul 30 2020, 4:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 30 2020, 4:33 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
atrosinenko requested review of this revision.Jul 30 2020, 4:33 AM
atrosinenko edited the summary of this revision. (Show Details)Aug 12 2020, 1:48 AM

Ping

sepavloff added inline comments.Aug 17 2020, 11:01 PM
compiler-rt/test/builtins/Unit/divdf3_test.c
29–34

Expressions `any/nan' also could be checked.

49–54

Expression `Inf/0' also could be checked.

92

Is 0x1.00000001p+0 equal to 1.0 in UQ1.31?

compiler-rt/test/builtins/Unit/divsf3_test.c
27

Similar notes as for double precision checks.

compiler-rt/test/builtins/Unit/divtf3_test.c
34–36

Similar notes as double precision checks.

Address the review comments.

@sepavloff

Thank you for the test cases. Looks like it is worth completely rewriting the three tests as table-driven tests, so for example adding four cases for [+-]Inf / [+-]0.0 would be almost as easy as testing only one of them (something similar is already implemented for udivmod?i4_test.c). The differences between adjacent test cases would be more visually obvious, as well.

compiler-rt/test/builtins/Unit/divdf3_test.c
92

Divisor is 1.(31 zeroes)1 after restoring the implicit bit, so it is truncated to 1.0 as UQ1.31. Instead of counting bits carefully, it would probably be better to add several tests with the 1 bit shifted 1-2 places left/right as well as if the divisor is round up instead of truncating - just in case. :) So, with table-driven test it would probably be simpler to not make extra assumptions on the implementation.

@sepavloff

Thank you for the test cases. Looks like it is worth completely rewriting the three tests as table-driven tests

Tests are not required to be compact. It is more important to have clear checks, which can be easily extracted. Table-driven tests are suitable if number of test cases is large, I think this is not the case.

compiler-rt/test/builtins/Unit/divdf3_test.c
92

Divisor is 1.(31 zeroes)1

So it is not 1.0 and the comment is misleading. Try rewording the comment to avoid confusion. Maybe divisor is truncated to 1.0 in UQ1.31 or something like that.

Rebase the entire patch stack against the up-to-date master and re-upload.

Fix the misleading comment.

compiler-rt/test/builtins/Unit/divdf3_test.c
92

Now got it, thank you.

This revision is now accepted and ready to land.Aug 25 2020, 3:41 AM

Uploaded, thank you!