[m-rev.] for review: make many integer casts into builtins

Zoltan Somogyi zoltan.somogyi at runbox.com
Sun May 27 07:51:59 AEST 2018

On Thu, 24 May 2018 21:32:29 -0400 (EDT), Julien Fischer <jfischer at opturion.com> wrote:

> Hi Zoltan,
> On Thu, 24 May 2018, Zoltan Somogyi wrote:
> > 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.
> Please do *not* do this.

Actually, it was the option I thought of first. Then, after about ten milliseconds,
I thought through the implications, and recoiled like you did :-)

> > 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.)
> We don't have casts from floats to ints.  If you want to convert
> from a float to an int you need to use one of
> {ceiling,floor,truncate,round}_to_int functions; having the conversion
> require programmer to state how the fractional part of the float be
> dealt with is a good thing in my view.

No, we don't have float to int casts. But if we took option C, it would require
the final code generator to know what it should do if it is ever given
an LLDS or MLDS cast operation with source=float and target=int.
That is my main reason for favouring option D over C.

> This raises another couple of issues: the documentation of those
> functions (e.g. floor_to_int etc) is not about what happens in a number
> of edge cases (e.g. if the argument won't fit in an int or is one of
> the special floating point values).
> The behaviour of the int->float conversion (i.e. float/1) is also
> underspecified and prone to get a bit interesting at the edge cases.

Agreed, but those are separate issues from the one under consideration.

> > 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?
> I think D is fine.

I will go with that then.


More information about the reviews mailing list