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.
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
Configure-time option for float array optimization. #1294
Conversation
I firmly believe that this is the right approach (and was planning to make a pull request that did just this change anyway). |
Oh yes that's right. I had to add them to the |
stdlib/array.ml
Outdated
external create : int -> floatarray = "caml_floatarray_create" | ||
external get : floatarray -> int -> float = "caml_floatarray_get" | ||
external set : floatarray -> int -> float -> unit = "caml_floatarray_set" | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need more functions here to handle this type (map, fold, etc), right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. That's the user-facing part that I want to do in another PR because it will take longer to discuss and it's not really needed for benchmarking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK sounds good.
byterun/alloc.c
Outdated
@@ -214,7 +218,11 @@ CAMLprim value caml_alloc_dummy_function(value size,value arity) | |||
/* [size] is a [value] representing number of floats. */ | |||
CAMLprim value caml_alloc_dummy_float (value size) | |||
{ | |||
#ifdef FLAT_FLOAT_ARRAY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not look correct to me. caml_alloc_dummy_float is used to compile rec-bindings, in two cases: (i) unboxed float arrays; (ii) unboxed float records. AFAICT, (i) cannot happen any more (there are not Floatarray litterals) but (ii) can still happen. So I think caml_alloc_dummy_float should be left unchanged.
byterun/alloc.c
Outdated
@@ -229,12 +237,17 @@ CAMLprim value caml_update_dummy(value dummy, value newval) | |||
CAMLassert (tag < No_scan_tag || tag == Double_array_tag); | |||
|
|||
Tag_val(dummy) = tag; | |||
#ifdef FLAT_FLOAT_ARRAY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above. caml_update_dummy might still be called (for unboxed float record) on a Double_array_tag block, no?
byterun/array.c
Outdated
else | ||
return caml_array_get_addr(array, index); | ||
#endif | ||
CAMLassert (Tag_val(array) != Double_array_tag); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about putting the assertion under a #elseif to avoid it in legacy mode? (But perhaps the C compilers are clever enough to avoid it.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the point? The assertion is also true in legacy mode, and more assertions are always better. Assertions are only compiled in the debug runtime anyway.
stdlib/array.ml
Outdated
@@ -30,6 +30,12 @@ external unsafe_blit : | |||
external create_float: int -> float array = "caml_make_float_vect" | |||
let make_float = create_float | |||
|
|||
module Floatarray = struct | |||
external create : int -> floatarray = "caml_floatarray_create" | |||
external get : floatarray -> int -> float = "caml_floatarray_get" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is WIP, but do we agree that this should be a %-primitive instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It definitely should. Otherwise it will be very slow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes of course.
@@ -30,6 +30,12 @@ external unsafe_blit : | |||
external create_float: int -> float array = "caml_make_float_vect" | |||
let make_float = create_float | |||
|
|||
module Floatarray = struct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the naming side, I don't like Array.Floatarray
. I'd prefer any of Array.FloatArray
, Array.Float
, Float.Array
, or a toplevel FloatArray
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of these I like Float.Array
best, since it's easiest to extend to additional types later (e.g. a specialized Int64.Array
or Bool.Array
).
Of course, we'd need a Float
module first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yallop Array.Float
has my preference, and I'd say it's just as easy to extend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@damiendoligez: I think type-specialized array representation is a bit like type-specialized compare
in this respect. Rather than enumerating all the type-specialized compare
functions in a single module, we've put each instance alongside its type: Int64.compare
, Char.compare
, etc.
Doing things that way has the advantage that it extends well to types outside the standard library, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, it would avoid awkward dependency cycles. Array is towards the start of the dependency chain right now. Adding Array.*
for each base type of the stdlib would need to move it towards the end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Float.Array
is most satisfactory. It matches up with existing usage in the compiler of Foo.Map
, Foo.Set
, etc.
byterun/array.c
Outdated
return caml_array_get_addr(array, index); | ||
} | ||
|
||
CAMLprim value caml_floatarray_get(value array, value index) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As said in another note, I believe this should become a %-primitive and be handled directly by the compiler.
byterun/array.c
Outdated
return caml_array_set_addr(array, index, newval); | ||
} | ||
|
||
CAMLprim value caml_floatarray_set(value array, value index, value newval) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, this should be a %-primitive I think.
byterun/caml/mlvalues.h
Outdated
#define Double_field(v,i) Double_field_flat(v,i) | ||
#define Store_double_field(v,i,d) Store_double_field_flat(v,i,d) | ||
#else | ||
static inline double Double_field (value v, mlsize_t i) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the need for Double_field is only for the transition period, to allow C stubs to be compatible with both modes. In a world without the legacy mode, one would not need it, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm clarifying this part by adding comments, and I'm finding bugs and redundancies. Stay tuned for a new commit soon.
byterun/interp.c
Outdated
@@ -645,7 +645,8 @@ value caml_interprete(code_t prog, asize_t prog_size) | |||
Instruct(ATOM): | |||
accu = Atom(*pc++); Next; | |||
|
|||
Instruct(MAKEBLOCK): { | |||
Instruct(MAKEBLOCK): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This stylistic change is at odds with other similar cases around.
Firstly: hoorah! See some inline comments (note: I haven't reviewed everything). Some other general points:
|
Beyond that it would also allow generalizing all covariant type variables, rather than just "strongly covariant" ones. |
I'd love some explanation of this for those of us not as well-versed in type theory. First, @garrigue's comment: how does the float optimization prevent us from using top (the universal supertype)? Does it apply only to subtyping in OCaml ie. row types? Second, @lpw25's comment: what is the current limitation with regard to strong covariant types? |
1eed20d
to
485cd65
Compare
New version with bugs removed and primitives added. @alainfrisch |
bytecomp/translcore.ml
Outdated
@@ -413,6 +416,16 @@ let specialize_primitive p env ty ~has_constant_constructor = | |||
| (Parraysetu Pgenarray, p1 :: _) -> Parraysetu(array_type_kind env p1) | |||
| (Parrayrefs Pgenarray, p1 :: _) -> Parrayrefs(array_type_kind env p1) | |||
| (Parraysets Pgenarray, p1 :: _) -> Parraysets(array_type_kind env p1) | |||
| (Parraylength Paddrarray, [p]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this chunk needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As evidenced by testsuite/tests/translprim/array_spec.ml
, without this code we lose some type specialization: without -no-flat-float-array, the compiler will specialize some accesses from gen
to int
, while with the option they are not gen
but addr
, and the compiler doesn't specialize them. This block of code recovers this optimization in the -no-flat-float-array case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does this specialization from gen to int happen? Woudl'nt this be the place to change to make it happen from addr to int as well? (Sorry if I miss something obvious.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well... it happens right here in this pattern matching.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, yes, sorry. I would find it easier to follow if the cases were merged:
| (Parraylength old_kind, [p]) -> Parraylength(cap_array_kind old_kind (array_type_kind env p))
asmrun/spacetime_snapshot.c
Outdated
@@ -259,6 +259,8 @@ static value take_snapshot(double time_override, int use_time_override) | |||
CAMLassert(sizeof(double) == sizeof(value)); | |||
v_time = allocate_outside_heap_with_tag(sizeof(double), Double_tag); | |||
Double_field(v_time, 0) = time; | |||
/* CR doligez: either this will have to change for no-flat-float-array, | |||
or the ML-side type will change to floatarray */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are just supposed to be floats, not float arrays -- I think I may have used the wrong macro here. Could you fix it? (I think you can remove the CRs then.)
@@ -351,6 +351,7 @@ utils/config.ml: utils/config.mlp config/Makefile | |||
-e 's|%%WITH_PROFINFO%%|$(WITH_PROFINFO)|' \ | |||
-e 's|%%WITH_SPACETIME%%|$(WITH_SPACETIME)|' \ | |||
-e 's|%%WITH_SPACETIME_CALL_COUNTS%%|$(WITH_SPACETIME_CALL_COUNTS)|' \ | |||
-e 's|%%FLAT_FLOAT_ARRAY%%|$(FLAT_FLOAT_ARRAY)|' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be more descriptive and less potentially confusing, for the C defines and the variable in Config
, to just use float_array_hack
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't want to use "hack" because it has a negative connotation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would argue that the most important thing is to have a name that really won't be confused with anything else. There are various different concepts in the code now relating to various kinds of "float array"s and the risk of accidental confusion is I suspect quite high. The "float array hack" may not be the ideal term, but it is the standard terminology, and so seems the right thing to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mshinwell: I do find the "hack" terminology offensive, and it is "standard" only in your mind. Do you still want to argue pointlessly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only other option I could think of was float array optimization, but then you need to contract it to float_array_opt or some such thing and it can be confused with option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(The thinking is that maybe "unboxed" is a more common term for this kind of representation than "flat". I suppose if doing that it would make sense to rename "flat" to "unboxed" throughout the patch.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before I start renaming everything, @xavierleroy do you agree with unboxed
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MAGICALLY_UNBOXED_FLOAT_ARRAY? (It's not like we're getting rid of unboxed float arrays.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intention of UNBOXED_FLOAT_ARRAY
(same for FLAT_FLOAT_ARRAY
, I imagine, too) is that it means "unboxed float array
" (as opposed to "unboxed floatarray
"). As such I'm not sure any extra qualification is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But they're both unboxed/flat, and that's where the confusion stems from. The difference is that one is demarcated by the type system, and one isn't.
let res = Array.Floatarray.create (List.length fields) in | ||
List.iteri (fun i f -> Array.Floatarray.set res i (float_of_string f)) | ||
fields; | ||
Obj.repr res |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two things:
- Can we change
Array.Floatarray
toFloat.Array
? (Then we can also addFloat.pi
which will close another GPR too.) - This is
Float.Array.of_list
, let's just move the code to the stdlib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not yet, because there is no Float
module in the stdlib. But I agree this naming is temporary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we can't introduce a Float
module? Otherwise we'll be stuck with Array.Floatarray
for evermore, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, in this PR it contains only the minimum needed to patch the compiler and to experiment with the new type. No reason we can't move or duplicate it to a proper stdlib module when we decide the option should be the standard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
#define Setup_for_gc | ||
#define Restore_after_gc | ||
Alloc_small(res, Double_wosize, Double_tag); | ||
#undef Setup_for_gc | ||
#undef Restore_after_gc | ||
Store_double_val(res, d); | ||
return res; | ||
#else /* FLAT_FLOAT_ARRAY */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is confusing -- it looks like the "else" branch is when the float array hack is enabled, which it's not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This has been addressed)
byterun/array.c
Outdated
@@ -42,39 +45,72 @@ CAMLexport int caml_is_double_array(value array) | |||
return (Tag_val(array) == Double_array_tag); | |||
} | |||
|
|||
/* [ 'a array -> int -> 'a ] where 'a != float */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this comment is wrong when the float array hack is disabled: in that case, 'a
can be float
. (At least one other occurrence of this below.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These comments are intended to be true in both cases, as a guide to write C code that works with and without the option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you perhaps clarify the comments along those lines? I think it's potentially misleading at the moment.
} | ||
CAMLreturn (res); | ||
} | ||
} | ||
#else | ||
return init; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be worth a comment explicitly noting that a copy is not required, since the name of the function makes it sound like it should always copy. Or maybe you should change the name of the function, in fact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This has been addressed)
if (!vv || ft == Forward_tag || ft == Lazy_tag || ft == Double_tag){ | ||
if (!vv || ft == Forward_tag || ft == Lazy_tag | ||
#ifdef FLAT_FLOAT_ARRAY | ||
|| ft == Double_tag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also need to update the comments in stdlib/lazy.ml
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This has been addressed)
byterun/interp.c
Outdated
@@ -723,6 +723,8 @@ value caml_interprete(code_t prog, asize_t prog_size) | |||
accu = Field(accu, *pc); pc++; Next; | |||
Instruct(GETFLOATFIELD): { | |||
double d = Double_field(accu, *pc); | |||
/* CR doligez: change to Double_flat_field ? Need to check how the | |||
compiler uses this instruction. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these are only used for accesses to values known to have tag Double_array_tag
. (So yes.)
#define Double_field(v,i) Double_val((value)((double *)(v) + (i))) | ||
#define Store_double_field(v,i,d) do{ \ | ||
|
||
/* The [_flat_field] macros are for [floatarray] values and float-only records. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhere there should be a note observing that record types "obviously of all floats" are still represented using tag Double_array_tag
. There may otherwise be a misconception that this optimisation has also been removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This has been addressed)
@mshinwell @alainfrisch I have changed the code according to most of your comments and answered the others. |
I will read the new diff first thing tomorrow, Friday. |
byterun/interp.c
Outdated
double d = Double_field(accu, *pc); | ||
/* CR doligez: change to Double_flat_field ? Need to check how the | ||
compiler uses this instruction. */ | ||
double d = Double_flat_field(accu, *pc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you missed the SETFLOATFIELD
case (below).
I wonder if it would be possible to use a limited form of monomorphization to bring up the performance of polymorphic code, at least when the entire program is available. OCaml as a language does not allow full monomorphization, but in many cases I suspect that it should be possible. |
It's not just a question of performance, though. There is also typechecker complexity and runtime-system complexity. It also spills into the type system itself (see the remark about the top type) and in strange places like restrictions on |
@damiendoligez For avoidance of doubt I'm happy with this now once you fix the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK subject to SETFLOATFIELD
fix as per other comment
Not sure about this claim. I don't think it's practical in OCaml, but it's Rust's approach, and Rust is the fastest language out there right now. They have other tricks up their sleeves, but this is one of the big ones. |
This all looks fine to me now. |
950dd0b
to
eb21925
Compare
What are the next steps?
|
I'm pretty sure unboxing floats in arrays is not the source of Rust's performance. In fact Rust doesn't do the suggested optimisation -- they monomorphise everything but they don't use that to optimise unboxing, the user indicates what data to unbox by hand themselves. |
This statement is probably confusing. I mean they do not take advantage of monomorphisation to optimise the layout of data in memory, a la Mlton. Instead the user has complete control over how data is represented. |
@alainfrisch The next step is benchmarking. |
Did you merge on prupose? At least we should say that |
Will we be ok with breaking software using |
We can keep Array.Floatarray with a |
Ok, I missed that Array.Floatarray is explicitly excluded from the documentation, and should thus not be relied upon.
Do you agree that benchmarking numerical code is rather useless (performance sensitive code bases make sure to avoid generic accesses, and will need to be rewritten to use |
I disagree. I'd like to see numbers for OCaml code as it exists today in the wild, not for future rewrites of this code. That will help determining how much code needs rewriting, among other things.
I agree we need numbers both for numerical and symbolic code. |
As an aside, with floatarray code, it should be relatively easy to start generating vectorized instructions, which should significantly improve performance. Perhaps we want to create annotations for this purpose as a simple first measure. |
No easier than today with |
Correct.
One of Rust’s biggest wins is that everything is (by default) a value type
with move semantics. This includes mutable ones, which can avoid boxing.
It would be much easier to write high-performance OCaml if it had
monomorphization and value types. OCaml's type system doesn't allow
full monomorphization,
but I suspect that some would be possible if the entire program was
available.
(Rust has other tricks up its sleeve, too. For instance, every
higher-order function that takes a closure generically - most do - has a
fresh copy generated specialized to each closure passed to it. That,
combined with no implicit boxing, means that higher order Rust often has
zero performance penalty).
…On Sep 15, 2017 12:20 PM, "Leo White" ***@***.***> wrote:
they monomorphise everything but they don't use that to optimise unboxing
This statement is probably confusing. I mean they do not take advantage of
monomorphisation to optimise the layout of data in memory, a la Mlton.
Instead the user has complete control over how data is represented.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1294 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AGGWB0HeFO9cJZQXGZdCa_7RaXvlghgeks5siqNSgaJpZM4O7Thf>
.
|
@gasche suggested to try Coq with the
I guess Coq could also really benefit from some more |
@ejgallego I should have given more information, sorry. I think that the (You can test either (4.06 -flat-float-array against 4.06 -no-flat-float-array) or (4.06 against 4.06 -no-flat-float-array).) |
Indeed sorry, I got it right at first but then got confused when feeding my bench script. Relaunching. |
No significative change in timing that I could observe when using |
cc @ppedrot: this is a flag to disable the float-array optimization (and correspondingly speedup generic array creation and access), and it preliminary testing suggests that it does not seem to change Coq performances (on the stdlib). Do you think there is a test that would reveal a speedup for Coq users? Do you know which OCaml functions are critically more expensive this way (maybe those don't take the flag into account properly)? Do we need to revert some of your manual library-specialization work to see the difference? |
@gasche I have added this particular flag to the main benchmark script so soon it will get tested against quite a few 3rd party developments, that should provide us with some more data as their performance profile is fairly diverse. |
Is this configuration used in some production environment (heavily tested)? |
This PR introduces a configure-time option to remove the float array optimization. Removing this optimization makes
float array
operations slower, but all other polymorphic array operations slightly faster. It also removes some restrictions onlet rec
and especially on unboxed types.This is only the minimal changes to the runtime and compiler to enable experimenting. What's missing is a new
Floatarray
module to give operations on the newfloatarray
type that represents flat-allocated arrays of unboxed float. Because it is user-visible, this new module will generate a lot of discussion on naming, which is not yet relevant: we should first run some benchmarks on this PR to decide if we want to shift the trade-off away from float array optimization.See also #163 #1169
This PR hat two commits, you will need to bootstrap (
make core && make coreboot
) after applying the first commit.The test suite passes on my machine, but I haven't tried OPAM packages yet and I don't know how many libraries with C bindings will be impacted.
There is one design decision that should be discussed: should the
Obj.field
andObj.set_field
functions remain ad hoc (i.e. work on both regular arrays and flat float arrays) or should they be low-level primitives that assume regular arrays (and then we need to introducedouble_field
andset_double_field
) ? I've done the latter but this is open for discussion.