[m-rev.] for review: add builtin 8, 16 and 32 bit integer types -- Part 2

Zoltan Somogyi zoltan.somogyi at runbox.com
Mon Aug 21 04:09:04 AEST 2017



On Mon, 21 Aug 2017 01:03:35 +1000 (AEST), Julien Fischer <jfischer at opturion.com> wrote:
> > If the current state of bootstrapping allows you to replace each cast_from_int(0)
> > with 0i8, 0u8 etc, then please do so, since that would make all the returned values
> > compile time constants.
> 
> This is the change that allows the compile to recognise 0i8 etc. so that's
> not possible.  I will replace the calls to cast_from_int above and elsewhere
> once this has bootstrapped.

So everyone: install the compiler soon after Julien commits this change.

> > > +to_int8(Integer, Int8) :-
> > > +    Integer = i(_Sign, [Digit]),
> > > +    int8.from_int(Digit, Int8).
> > 
> > The extremely scant documentation in integer.m on the meaning of the representation
> > it uses makes me unable to determine whether ignoring _Sign here is a bug or not.
> > Someone else who does know this representation should review the changes to integer.m.
> 
> It's not a bug; the the digits are also signed.

Then what is the _Sign for?

Can you please document the meaning of integer.m's representation?
As part of another change, of course.

> > > +put_int8(Stream, Int8, !State) :-
> > > +    ( if
> > > +        % Handle the common I/O case more efficiently.
> > > +        dynamic_cast(!.State, IOState0),
> > > +        dynamic_cast(Stream, IOStream)
> > > +    then
> > > +        io.write_int8(IOStream, Int8, unsafe_promise_unique(IOState0), IOState),
> > > +        ( if dynamic_cast(IOState, !:State) then
> > > +            !:State = unsafe_promise_unique(!.State)
> > > +        else
> > > +            error("stream.string_writer.put_int8: unexpected type error")
> > > +        )
> > > +    else
> > > +        put(Stream, string.int8_to_string(Int8), !State)
> > > +    ).
> > 
> > Does this code really handle the I/O case more efficiently than just using the else case
> > for everything? It seems to have a significant amount of overhead, though I suppose it is
> > a constant amount of overhead, while the latter has overhead proportional to the length
> > of the string printed.
> 
> Simon added the above pattern for the builtin types many years ago.
> The justification was that in the presence of type specialization it
> makes io.write etc "almost" as fast as if it were written directly (i.e.
> if it were not implemented in terms of stream.string_writer).

Do you know whether this code *actually* gets type specialized?

> > These long if-then-else chains are slow.
> > 
> > We ought to have a system where we peek inside Univ, get back e.g. a string
> > identifying the type inside. We would then switch on that string, with
> > each switch arm doing a dynamic cast on only *one* type, the one returned
> > by the peek.
> 
> That's a good idea, although not one for this change.

I didn't mean it for this change.

Zoltan.


More information about the reviews mailing list