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

Zoltan Somogyi zoltan.somogyi at runbox.com
Thu May 24 12:01:50 AEST 2018



On Thu, 24 May 2018 11:11:27 +1000 (AEST), Julien Fischer <jfischer at opturion.com> wrote:
> > For review by Julien. The first part of the diff to compiler/options.m
> > is not intended to be committed, at least not now.
> 
> When are you intending to commit that change?

I think it would be preferable for people who aren't Mercury
developers if I made the improvements in argument packing
the default in one go, after I have finished them all. The outstanding
ones include packing the remote secondary tag with any initial
run of sub-word-sized arguments, reversing the position of the
fields (to make field N occupy bits in a more significant position
than field N+1, to make them comparable together if their
signedness permits it), and storing a primary tag, a local secondary
tag and all the arguments in one word if they all fit in there.

Automatic argument reordering, to make argument packing
more effective, would require significant changes to our "system"
of interface files, and so I don't intend to wait for that.

> I was originally trying to avoid the situation where the various
> fixed-size int modules import each other (they have to import int and
> uint).  In practice, this didn't work out, so the convention is that
> the signed modules can import their corresponding unsigned module but
> not the other way around.

That looks like a worthwhile goal. However, it looks like it would
require someone who wants to cast e.g. int8 to int32 to go via int.

> My intention with the use of the word "cast" in the conversion function
> names was that it should flag that there is a potential loss of
> information in the conversion (e.g. change of signedness, narrowing
> conversion etc.)  That's why the int64 module does not have a function
> named cast_from_int/1.  (Because you can always convert a Mercury int to
> an int64 *without* loss of information.)

I think that is a good reason for why only int64.m has a non-cast from_int
function. I also think that using "cast" in a name to mean "there may be
some loss of information" is fine. But I don't see those as a reason why
there shouldn't *also* be a cast_from_int function in int64.m, clearly
documented as being a synonym for the plain from_int function, and
NOT involving loss of information.

My reasoning is based on my experience with working with sets
in the Mercury compiler. We have several implementations, and they
weren't always compatible; they had minor differences in the names
of predicates, or some functionality was available as a predicate
in one set module but not another. This made it annoying to change
the representation of something from one kind of set to another.
I am trying to avoid this kind of annoyance when a programmer
wants to change ":- type my_type == int32" to ":- type my_type == int64".

> Do you have an alernative proposal?

My addition to your proposal would be: the same functionality should be
available from all eight modules (maybe all ten, including int.m and uint.m)
under the same name if possible, with the obvious "mutatis mutandis" changes,
such as int8.cast_from_uint8 versus uint8.cast_from_int8, and no casts
to and from int in int.m itself.

> > Do you think you should do that, or should I?
> 
> Let's work out what it should be first.

Agreed.

> The diff is fine BTW.

Even including the addition of cast_from_int to library/int64.m?

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
tomorrow.

Zoltan.


More information about the reviews mailing list