[m-rev.] for review: make many integer casts into builtins
zoltan.somogyi at runbox.com
Thu May 24 23:33:48 AEST 2018
On Thu, 24 May 2018 04:01:50 +0200 (CEST), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> In any case, I am testing it in 32 bit mode before committing it. And a
> good thing too: I have found two test case failures in asm_fast.gc, but
> I don't know yet if they are caused by this diff or not. I will look at it
I have isolated the problem, which was the same for both test failures.
It raises a nontrivial question that was foreseen in a comment in mlds.m
for a while now.
The problem was that when casting to e.g uint64, it matters what type
you are casting *from*. If you are casting from int64, then a simple
"(uint64_t)" prefix is all you need in C. However, if you are casting
from a *boxed* int64 value, then you need to call the MR_word_to_uint64
macro, which will do the unboxing for you on 32-bit platforms, while
doing the same cast as above on 64-bit platforms.
None of this is visible in the foreign_procs implementing the cast functions
in e.g. uint64.m. Their C codes have just the casts; the calls to e.g.
MR_word_to_int64 and MR_uint64_to_word are added automatically by
(Note that this meant that on 32-bit platforms, casting an int64 to uint64,
or vice versa, currently incurs the penalty of a heap allocation, and the
copying of the old memory cell's contents to the new heap cell; when
just returning the pointer to the existing cell would be good enough.)
- casts between intN and uintN are OK for all values of N;
- casts between int and intN are OK if N =< 32, and likewise for uints; but
- casts between int and int64 require using MR_word_to_int64 or MR_int64_to_word,
and likewise for uints.
As far as this diff is concerned, there are several possible avenues for fixes.
Option A is to simply not make the third category of operations into builtins.
This is simple to do, but leaves performance on the table.
Option B is to make the third category of operations into builtins only if the
target machine is 64-bit. This is *far* from simple to do, because it means
that whether a function is a builtin or not depends on the target platform,
which means e.g. that we won't know whether to reject clauses for them.
Option C is to generalize all three notions of cast in the compiler (in the LLDS,
in the MLDS, and in builtin_ops.m itself) to include the source type as well
as the destination type. This is a lot of work, in large part because it requires
deciding what to do for sort-of-but-not-really sensible casts, such as casts
straight from floats to uint16. (For obviously-not-sensible casts, such as
float to type_info, an abort is obviously fine.)
Option D: add MR_word_to_int64, its inverse and their uint64 variants
as separate operations to builtin_ops.m. I am not sure yet whether they will need
modifications to the LLDS and/or the MLDS rval types; the code generators
should be able to implement them as a combination of box/unbox and cast operations.
My preference is option D. What are your opinions?
More information about the reviews