Add support for long double and long double complex #475

andrewray · 2016-11-08T03:13:36Z

Adds two new types to ctypes - ldouble and complexld.

ldouble is an alias for Ldouble.t which is implemented as
a C custom_operation in ldouble_stubs.c. A tiny API to convert
to/from floats is provided.

complexld is an alias for Ldouble.complex. Unlike the other
complex types it does not map to the standard Complex.t type
(I dont think it would be very useful if it did as you would
immediately loose the added precision). A tiny API to construct
and extract the long double real and imaginary components is
provided.

A few tests have been updated - most importantly the test-complex
one which shows long double complex in action with both the FFI
and Cstubs.

Issues;

no hash or (de)serialize for these types. Not sure how to
implement these
The Ldouble APIs should probably be fleshed out for convenience.
Basic arithmetic and to/of string.
No nice printing of values (need to update ctypes-top).
Needed to re-run make depend but that seems to be broken so
there is a tiny hack in the makefile (see PROJECTS_DEP). Old/missing
project definitions are causes multiple iempty -I -I -I ... args to
ocamldep.
should complexld really live in Ldouble?

yallop · 2016-11-09T15:51:30Z

Thanks! I'd like to merge this, once the issues are worked out. First:

Needed to re-run make depend but that seems to be broken so there is a tiny hack in the makefile (see PROJECTS_DEP). Old/missing project definitions are causes multiple iempty -I -I -I ... args to ocamldep.

I've fixed this slightly differently in #478, filtering out empty directories from the PROJECTS list. In the interest of keeping the diff small here, could you try rebasing on top of that, once it's merged?

andrewray · 2016-11-09T18:11:46Z

I've done the rebase (my git-fu is weak here, but it seemed to work). I've tried to get the .depend file changes down a bit as well (by using ocaml 4.04), but can't seem to manage it

andrewray · 2016-11-09T21:25:26Z

I've hand massaged the .depend file to try and tidy up the commit. It seems the reason it was quite so different is related to the find command in the depend target which seems to produce files in different orders on different machines. It might be worth adding a sort step here to avoid this in the future.

yallop · 2016-11-10T11:17:03Z

That .depend patch looks much better! I wonder if passing the -sort option to ocamldep would help with the ordering issue. (I don't think it's worth spending much time on it now, though, since it'll be fixed when we switch to ocamlbuild (#451).)

yallop · 2016-11-10T11:30:51Z

no hash or (de)serialize for these types. Not sure how to implement these

The code in the OCaml distribution for hashing and serializing doubles should be a useful starting point, but it's complicated by the fact that long double has different sizes on different platforms.

Perhaps it's sufficient to conditionalize on LDBL_MANT_DIG, which is likely to be either 52 (i.e. the same as double, which is the case with Visual C++), 64 (on gcc/x86_64), or 112 (on a few other platforms).

andrewray · 2016-11-10T12:02:14Z

I made a bash at hashing last night. I note that in some places the ocaml distribution goes to some lengths to make the differences in data types between platforms transparent. Do we need to go that far here? I think writing (and testing) all the required conversions for this would be quite difficult.

yallop · 2016-11-10T12:26:22Z

One thing to worry about is that there are platforms where long double contains non-value bits. For example, with gcc/x86_64 long double has 80 value bits, but the storage size is 128 bits, and the extra bits can be filled with junk.

Here's an example program that illustrates the difference between accessing long doubles as values and accessing them as byte sequences on such a platform:

#include <stdio.h>
#include <string.h>

void print_hex_longdouble(const void *q)
{
  const unsigned char *p = q;
  for (size_t i = 0; i < sizeof(long double); i++)
    printf("%.2x", (unsigned)p[i]);
}

int main()
{
  long double x = 1.0, y = 1.0;

  printf("x: %LA, y: %LA\n", x, y);
  printf ("x == y: %d\n", x == y);

  printf ("x: 0x");   print_hex_longdouble(&x);
  printf ("\ny: 0x"); print_hex_longdouble(&y);
  printf ("\n");

  printf("memcmp(x,y): %d\n",
     memcmp(&x, &y, sizeof(long double)));
}

And here's the output of the program on my machine:

$ gcc -std=c99 -pedantic -W -Wall ldbl.c 
$ ./a.out 
x: 0X8P-3, y: 0X8P-3
x == y: 1
x: 0x0000000000000080ff3f000000000000
y: 0x0000000000000080ff3f4ee94d560000
memcmp(x,y): -78

So a hashing implementation that uses all the bytes of a long double may return different hashes for equal values.

yallop · 2016-11-10T14:18:34Z

Do we need to go that far here?

If it does turn out to be too much work, I think it's fine to leave long doubles as unhashable/unserialisable for now, and open an issue as a reminder to add implementations later.

andrewray · 2016-11-10T18:03:53Z

I think I'd like to try and get hashing and serialization working for the various supported platforms, but avoid dealing with any inter-platform compatibility stuff, as that seems overly complicated.

I note a failure on mingw64 at the moment which I will try to investigate (some internet comments suggest long double, mingw and msvc runtime do not play well together, but we'll see).

andrewray · 2016-11-12T22:24:06Z

With the latest commits I am pretty happy with this patch.

It considers directly 4 different long double representations; double, __float80, __float128 and __ibm128.
If some other case comes up it will just convert to double at hash/serialization time.
- note; of course this would likely loose precision
- note; always converting to double for serialization would be one simple way to ensure compatibility between platforms at the cost of reduced precision.
I have been able to test the __float80 version on intel the most. I have checked that hashing and serialization is consistent between linux 64 bit, 32 bit and windows 64 bit.
As a wee hack I forced linux 64 bit to use the double conversion code and checked that serialization and hashing were ok. It would be nice if this could be properly checked with the ARM based CI stuff.
For __float128 and __ibm128 I wrote a bit of c-code to test the hashing code between ppc64 and ppc64le with qemu. The serialization code is untested (setting up a working ppc emulator and ocaml system was a bit too much hassle).

yallop · 2016-11-14T15:28:02Z

Thanks! It's going to take me a little while to review this, but I'll aim to get to it over the next few days.

Would you mind if I cherry-picked some of the unrelated changes (e.g. fixes to the sizeof tests, and useconds_t printing) to keep this self-contained? Alternatively, separate PRs for those changes would be very welcome.

andrewray · 2016-11-15T09:58:51Z

I'll send a PR for the sizeof fix (there is no fix for useconds_t, just a moved list delimiter).

If you like I could also squash all the commits except the 1st. This would split the patch into the bit related to ctypes internals and the Ldouble API.

Adds two new types to ctypes - `ldouble` and `complexld`. `ldouble` is an alias for `Ldouble.t` which is implemented as a C custom_operation in `ldouble_stubs.c`. A tiny API to convert to/from floats is provided. `complexld` is an alias for `Ldouble.complex`. Unlike the other complex types it does not map to the standard Complex.t type (I dont think it would be very useful if it did as you would immediately loose the added precision). A tiny API to construct and extract the long double real and imaginary components is provided. A few tests have been updated - most importantly the test-complex one which shows long double complex in action with both the FFI and Cstubs. Issues; * no hash or (de)serialize for these types. Not sure how to implement these * The Ldouble APIs should probably be fleshed out for convenience. Basic arithmetic and to/of string. * No nice printing of values (need to update ctypes-top). * should complexld really live in Ldouble?

yallop

Thanks -- this looks very good!

I'd like to merge once the suggested changes (which I hope aren't too onerous) are made.

yallop · 2016-11-18T15:27:27Z

Makefile.tests

@@ -1215,5 +1224,5 @@ test: build testlib tests-common $(TESTS) \

 run-%:	$*
 	@echo running $*
-	@cd $(BUILDDIR) && CAML_LD_LIBRARY_PATH=. LD_LIBRARY_PATH=clib DYLD_LIBRARY_PATH=clib ./$*.$(BEST) -runner sequential
+	@cd $(BUILDDIR) && CAML_LD_LIBRARY_PATH=. LD_LIBRARY_PATH=clib DYLD_LIBRARY_PATH=clib ./$*.$(BEST) -runner sequential #$($*.test_args)


This line appears to be unchanged, except for a commented-out part. Can it be reverted?

yallop · 2016-11-18T15:28:32Z

src/cstubs/cstubs_public_name.ml

@@ -32,6 +32,8 @@ let ident_of_ml_prim : type a. a Ctypes_primitive_types.ml_prim -> path =
   | ML_ullong -> path_of_string "Unsigned.ullong"
   | ML_ulong -> path_of_string "Unsigned.ulong"
   | ML_ushort -> path_of_string "Unsigned.ushort"
+   | ML_ldouble -> path_of_string "Ldouble.t"
+   | ML_complexld -> path_of_string "Ldouble.complex"


(These will need to change if the module name is changed.)

yallop · 2016-11-18T15:28:58Z

src/ctypes/complex_stubs.c

@@ -43,3 +43,4 @@ double complex ctypes_double_complex_val(value v)
 {
  return Double_field(v, 0) + Double_field(v, 1) * I;
 }
+


The only change here is a blank line; please revert.

yallop · 2016-11-18T15:29:49Z

src/ctypes/ldouble.mli

+(** The type of long doubles. *)
+
+val to_float : t -> float 
+(** Convert a long double to a float *)


This should document the behaviour on overflow -- ideally to raise an exception.

yallop · 2016-11-18T15:29:51Z

src/ctypes/ldouble.mli

+(** Create a long double from a float *)
+
+val to_int : t -> int 
+(** Convert a long double to an int *)


This should document the behaviour on overflow -- ideally to raise an exception.

yallop · 2016-11-18T15:43:56Z

src/ctypes/ldouble.mli

@@ -0,0 +1,258 @@
+(*


A couple of changes to the naming and module organization would bring this in line with ctypes conventions.

First, calling the module LDouble rather than Ldouble would match ULong, UInt32, etc.

Second, it'd be better to have LDouble.Complex at the same level as Ldouble rather than nested underneath; it could perhaps be called ComplexL or somesuch.

(I don't have a strong preference as to whether LDouble and Complex live at the top level (like Int32 and Int64) or under a parent module such as Floats (like Unsigned.ULong)).

yallop · 2016-11-18T15:45:00Z

src/ctypes/ldouble.mli

+(* debugging only *)
+(*val to_hex_string : t -> string*)
+
+val add : t -> t -> t 


If you're so inclined, it would be nice to have an LDouble.Infix module with aliases (+, -, etc.) for the arithmetic functions (and similarly for the Complex module).

yallop · 2016-11-18T15:45:21Z

src/ctypes/ldouble.ml

+
+type complex
+
+module Complex = struct


Could this module please have a type t member?

yallop · 2016-11-18T15:45:53Z

src/ctypes/ldouble_stubs.c

+static void ldouble_serialize(value v, uintnat *wsize_32, uintnat *wsize_64) {
+  long double *p = Data_custom_val(v);
+  int size;
+  caml_serialize_int_1(LDBL_MANT_DIG);


I think this is a good idea.

yallop · 2016-11-18T15:46:45Z

src/ctypes/ldouble.mli

+(** The difference between [1.0] and the smallest exactly representable
+    floating-point number greater than [1.0]. *)
+
+val nan : t


There appear to be multiple nan values for ldouble, which isn't the case for from OCaml's float type:

# Ldouble.(compare (neg nan) nan);; - : int = 1 # (compare (-.nan) nan);; - : int = 0

andrewray · 2016-11-18T16:37:19Z

Regarding the format function - my reason for leaving it in is that it is the only way to generate a string with more (or arbitrary) precision if you need it. That said it is not a safe interface and should be fixed. Perhaps we could hide it behind safer interface such as

val to_string : ?digits:int -> ?prec:int -> t -> string

which would generate a C-format string like %<digits>.<prec>Lf?.

yallop · 2016-11-18T16:50:10Z

Yes, I like that approach better.

A slightly nicer approach than generating a format string is to pass digits and prec through to C, and then use "%*.*Lf" as the format string -- the * means that the width and precision are passed as arguments:

   snprintf(buf, len, "%*.*Lf", width, precision, d)

It's not quite as flexible (since you can't select between g and f with this approach), but it's probably sufficient.

andrewray · 2016-11-18T21:35:59Z

Regarding overflow for the various casting functions (to_float, to_int) I note that int_of_float just says the behaviour is undefined if out of range - is this acceptable (at least for now)?.

yallop · 2016-11-18T21:39:39Z

Yes, that's fine for now.

andrewray · 2016-11-18T22:59:54Z

I have split complex/long double into your suggested LDouble and ComplexL modules.

This properly needs a make depend run but I have just hacked in enough to make it compile from a make clean for now. I dont propose reorganising it by hand again! I would suggest getting this patch right then fixing this seperately.

yallop · 2016-11-19T11:47:47Z

I dont propose reorganising it by hand again! I would suggest getting this patch right then fixing this seperately.

That sounds good to me.

yallop · 2016-11-21T12:00:09Z

The revised patch looks very good. Thank you for all your work on this, @andrewray!

I'm going to rebase/squash this a bit and then merge.

Marshal tests added to test-ldouble. The implementation needs testing (and almost certainly fixing) for different endianness/ldouble sizes. Also added a to_hex_string function to show the underlying bytes, including unused ones, so I can check that the hash code is properly ignoring them.

- Seperate types into LDouble and ComplexL modules - document unspecified behaviour in conversion functions - make LDouble NANs work like floats - this also includes normalising NANs (and negative zeros) when serializing and hashing - tidy up LDouble to_string conversion - defend against bad of_string formats - (cheat) add a test to test-complex which passes/returns doubles Not addressed; Infix operators and dependancies are a bit broken. Dont use errno for error checking strtold This leads to problems with the Xen build on travis. Instead we perform the end pointer check and add a zero length string check.

whitequark · 2016-11-24T17:45:51Z

This PR broke Android because there is no cexpl/clogl/cpowl there.

andrewray force-pushed the ldouble branch 2 times, most recently from 3d8ed70 to 0876a68 Compare November 9, 2016 18:05

andrewray force-pushed the ldouble branch from 0876a68 to 92d9948 Compare November 9, 2016 21:20

andrewray force-pushed the ldouble branch from f1afca2 to 416e761 Compare November 15, 2016 20:29

yallop requested changes Nov 18, 2016

View reviewed changes

andrewray added 5 commits November 21, 2016 14:40

add hash for ldouble/complex

0dd69ca

add API and tests for ldouble

cccfcb1

Note a modf bug on mingw32.

aa00357

Add API and tests for Ldouble.Complex

71eca9e

yallop force-pushed the ldouble branch from 5a67b89 to 14f0674 Compare November 21, 2016 14:41

yallop approved these changes Nov 21, 2016

View reviewed changes

yallop merged commit c18ede8 into yallop:master Nov 21, 2016

yallop mentioned this pull request Nov 21, 2016

long double support #310

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for long double and long double complex #475

Add support for long double and long double complex #475

andrewray commented Nov 8, 2016

yallop commented Nov 9, 2016

andrewray commented Nov 9, 2016

andrewray commented Nov 9, 2016

yallop commented Nov 10, 2016

yallop commented Nov 10, 2016

andrewray commented Nov 10, 2016

yallop commented Nov 10, 2016

yallop commented Nov 10, 2016

andrewray commented Nov 10, 2016

andrewray commented Nov 12, 2016

yallop commented Nov 14, 2016

andrewray commented Nov 15, 2016

yallop left a comment

yallop Nov 18, 2016

yallop Nov 18, 2016

yallop Nov 18, 2016

yallop Nov 18, 2016

yallop Nov 18, 2016

yallop Nov 18, 2016

yallop Nov 18, 2016

yallop Nov 18, 2016

yallop Nov 18, 2016

yallop Nov 18, 2016

andrewray commented Nov 18, 2016 •

edited

Loading

yallop commented Nov 18, 2016

andrewray commented Nov 18, 2016 •

edited

Loading

yallop commented Nov 18, 2016

andrewray commented Nov 18, 2016

yallop commented Nov 19, 2016

yallop commented Nov 21, 2016

whitequark commented Nov 24, 2016 •

edited

Loading

Add support for long double and long double complex #475

Add support for long double and long double complex #475

Conversation

andrewray commented Nov 8, 2016

yallop commented Nov 9, 2016

andrewray commented Nov 9, 2016

andrewray commented Nov 9, 2016

yallop commented Nov 10, 2016

yallop commented Nov 10, 2016

andrewray commented Nov 10, 2016

yallop commented Nov 10, 2016

yallop commented Nov 10, 2016

andrewray commented Nov 10, 2016

andrewray commented Nov 12, 2016

yallop commented Nov 14, 2016

andrewray commented Nov 15, 2016

yallop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewray commented Nov 18, 2016 • edited Loading

yallop commented Nov 18, 2016

andrewray commented Nov 18, 2016 • edited Loading

yallop commented Nov 18, 2016

andrewray commented Nov 18, 2016

yallop commented Nov 19, 2016

yallop commented Nov 21, 2016

whitequark commented Nov 24, 2016 • edited Loading

andrewray commented Nov 18, 2016 •

edited

Loading

andrewray commented Nov 18, 2016 •

edited

Loading

whitequark commented Nov 24, 2016 •

edited

Loading