This is an archive of the discontinued LLVM Phabricator instance.

[COMPILER-RT] Implement long double comparison functions
ClosedPublic

Authored by koviankevin on Feb 13 2014, 9:45 PM.

Details

Summary

The following functions are implemented:
letf2, gttf2, netf2, getf2, eqtf2, lttf2, __unordtf2

Diff Detail

Event Timeline

koviankevin updated this revision to Unknown Object (????).Feb 19 2014, 6:11 PM

Since we use __uint128_t type, which is undefined in 32-bit machine toolchain, we enable these functions only when using 64-bit machine.

joerg added a comment.Feb 19 2014, 6:16 PM

That would be a problem as i.e. SPARC uses soft-float for long double.

We assume sizeof(long double)=16 (IEEE-754 128-bit format)
SIZEOF_LONG_DOUBLE=8 in SPARC and SIZEOF_LONG_DOUBLE=16 in SPARC64
SIZEOF_INT128 =16 is also defined in SPARC64, so these funcitons can be compiled in SPARC64

Could you specify the situation about when the problem occurs? Thanks

joerg added a comment.Feb 20 2014, 3:22 AM

OK, forget my comment. I misremembered the sate of 32bit SPARC. Ideally, __uint128_t would be supported on all targets, but that's a fundamentally different question.

koviankevin updated this revision to Unknown Object (????).Feb 24 2014, 7:43 PM

Use CRT_HAS_128BIT

joerg added a comment.Feb 25 2014, 3:44 AM

Can you add a define similar to CRT_HAS_128BIT to flag whether a platform should use this code? x86_64 uses 80bit long double, so it doesn't match. Checking LDBL_MANT_DIG == 113 or so would be a good first approximiation.

koviankevin updated this revision to Unknown Object (????).Feb 25 2014, 7:28 PM

introduce CRT_LDBL_128BIT

I will upload patches for other functions if my modifications are what you want, thanks.

joerg added a comment.Feb 26 2014, 1:11 PM

Missing test case, otherwise LGTM.

koviankevin updated this revision to Unknown Object (????).Feb 27 2014, 8:41 PM

add test

koviankevin updated this revision to Unknown Object (????).Mar 10 2014, 1:11 AM

modify by gribozavr's comment and use COMPILER_RT_ABI

Hi GuanHong Liu,

Thank you for working on this!

This set of patches looks good, but because I don't contribute regularly to compiler-rt, I will defer to other reviewers to stamp the final LGTM.

Dmitri

lib/builtins/comparetf2.c
70–76 ↗(On Diff #7674)

Could you sink the comment into the else {} block? It is not immediately obvious that the 'if' statement continues there.

koviankevin updated this revision to Unknown Object (????).Mar 12 2014, 8:23 PM

sink the comment

Hi GuanHong Liu,

I don't know if you noticed, but Joerg signed off on this patch, so please commit!

Dmitri

Hi Dmitri

I don't have commit authority, could you help me to commit them? thanks.

GuanHong

Joerg,

Since this patch depends on D2796, which is owned by you, I will defer to you on this patch.

Dmitri

I want to ask that whether our patches (including D2796, which is owned by Joerg now) can be commited?

Thanks,
GuanHong

joerg added a comment.Mar 28 2014, 3:39 AM

Sorry, I thought I had it committed already. No objections from me, but I can't test this.

Since I don't have commit authority, can you help me to commit other patches for quad precision? Thanks!
(i.e, D2797 to D2800, D2802 to D2805)

joerg closed this revision.Apr 1 2014, 6:50 AM

Closed by commit rL205312 (authored by @joerg).

Hi Joerg

Can I commit D2798, D2799, D2800, D2802, D2803, D2804 and D2805 ?
These patches are also implemented for IEEE quad precision.

Thanks,
GuanHong