Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid unaligned stores/loads #584

Merged
merged 1 commit into from
Dec 7, 2018
Merged

Avoid unaligned stores/loads #584

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 35 additions & 15 deletions src/ctypes/ldouble_stubs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <stdint.h>
#include <float.h>
#include <math.h>
#include <string.h>

#include "ctypes_ldouble_stubs.h"
#include "ctypes_complex_compatibility.h"
Expand Down Expand Up @@ -65,7 +66,11 @@
#define LDOUBLE_VALUE_BYTES LDOUBLE_STORAGE_BYTES
#endif

#define ldouble_custom_val(V) (*(long double *)(Data_custom_val(V)))
static inline long double ldouble_custom_val(value v) {
long double r;
memcpy(&r, Data_custom_val(v), sizeof(r));
return r;
}

// initialized in ldouble_init
static long double nan_;
Expand Down Expand Up @@ -155,21 +160,18 @@ static void ldouble_serialize(value v, uintnat *wsize_32, uintnat *wsize_64) {
*wsize_32 = *wsize_64 = sizeof(long double);
}

static int ldouble_deserialize_data(long double *q) {
static void ldouble_deserialize_data(long double *q) {
unsigned char *p = (unsigned char *)q;
if (LDOUBLE_VALUE_BYTES == 16) {
caml_deserialize_block_8(p, 2);
return 16;
} else if (LDOUBLE_VALUE_BYTES == 10) {
caml_deserialize_block_8(p, 1);
caml_deserialize_block_2(p+8, 1);
return 10;
} else {
double d;
if (sizeof(double) == 4) d = caml_deserialize_float_4();
else d = caml_deserialize_float_8();
*q = (long double) d;
return sizeof(double);
}
}

Expand All @@ -193,7 +195,7 @@ static struct custom_operations caml_ldouble_ops = {
value ctypes_copy_ldouble(long double u)
{
value res = caml_alloc_custom(&caml_ldouble_ops, sizeof(long double), 0, 1);
ldouble_custom_val(res) = u;
memcpy(Data_custom_val(res), &u, sizeof(u));
return res;
}

Expand Down Expand Up @@ -436,7 +438,12 @@ value ctypes_ldouble_size(value unit) {

/*********************** complex *************************/

#define ldouble_complex_custom_val(V) (*(long double _Complex*)(Data_custom_val(V)))
static inline long double _Complex ldouble_complex_custom_val(value v)
{
long double _Complex r;
memcpy(&r, Data_custom_val(v), sizeof(r));
return r;
}

static int ldouble_complex_cmp_val(value v1, value v2)
{
Expand All @@ -453,23 +460,36 @@ static intnat ldouble_complex_hash(value v) {

static void ldouble_complex_serialize(value v, uintnat *wsize_32, uintnat *wsize_64) {
long double re,im;
long double _Complex *p = Data_custom_val(v);
long double _Complex c;
void * p = Data_custom_val(v);
#if defined(__GNUC__) && __GNUC__ == 6 && __GNUC_MINOR__ == 4
/* workaround gcc bug. gcc tries to inline the memcpy calls, but
* fails with an internal compiler error. I've observed this error
* only under Alpine Linux, other distros have already imported a
* patch from upstream.
*/
void *(*volatile mymemcpy)(void*,const void*,size_t) = memcpy;
mymemcpy(&c, p, sizeof(c));
#else
memcpy(&c, p, sizeof(c));
#endif
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we ask the Alpine people to compile using -fno-builtin instead of fixing the GCC problem here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alpine is used by various CIs, including the ones by https://github.com/ocaml/opam-repository. A solution that isn't enabled by default would lead to frustration. I could move the check to the build system and add -fno-builtin there, but it would probably lead to even more lines of code.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Since the fix is constrained to a specific GCC release, it should at least be clear when we can safely remove it.

caml_serialize_int_1(LDBL_MANT_DIG);
re = ctypes_compat_creall(*p);
re = ctypes_compat_creall(c);
ldouble_serialize_data(&re);
im = ctypes_compat_cimagl(*p);
im = ctypes_compat_cimagl(c);
ldouble_serialize_data(&im);
*wsize_32 = *wsize_64 = sizeof(long double _Complex);
}

static uintnat ldouble_complex_deserialize(void *d) {
long double re, im;
int size;
long double _Complex c;
if (caml_deserialize_uint_1() != LDBL_MANT_DIG)
caml_deserialize_error("invalid long double size");
size = ldouble_deserialize_data(&re);
size += ldouble_deserialize_data(&im);
*(long double _Complex *)d = (ctypes_compat_make_complexl(re, im));
ldouble_deserialize_data(&re);
ldouble_deserialize_data(&im);
c = ctypes_compat_make_complexl(re, im);
memcpy(d, &c, sizeof(c));
return (sizeof(long double _Complex));
}

Expand All @@ -486,7 +506,7 @@ static struct custom_operations caml_ldouble_complex_ops = {
value ctypes_copy_ldouble_complex(long double _Complex u)
{
value res = caml_alloc_custom(&caml_ldouble_complex_ops, sizeof(long double _Complex), 0, 1);
ldouble_complex_custom_val(res) = u;
memcpy(Data_custom_val(res), &u, sizeof(u));
return res;
}

Expand Down