[m-rev.] for review: replace four "safe num digits" tables with one
Julien Fischer
jfischer at opturion.com
Tue Sep 15 23:54:23 AEST 2026
On Tue, 15 Sept 2026 at 23:34, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:
>
> On Tue, 15 Sep 2026 23:07:56 +1000, Julien Fischer <jfischer at opturion.com> wrote:
>
> > On Tue, 15 Sept 2026 at 20:33, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:
> > >
> > > For review by Julien. I used an awk script to merge the four old tables
> > > into the new table, so you don't need to check every entry in it.
> >
> > My only suggestion is that you have one table for each word size, as
> > the compiler
> > should be able to eliminate the unneeded table entirely.
>
> I marked the combined table predicate to be inline, and I checked, the compiler
> does do that. The code of each "can we use the fast path" predicate does
> a lookup in a single-column vector, because this is the result of
> - inlining the switch, each of whose arms returns four ints;
> - ignoring all but one of those ints in each of the 32 and 64 bit arms,
> - and then constant propagation (from the per-platform known word size)
> deleting the inapplicable arm.
If that's the case, then I have no objection to the big table.
...
> > > Unless you object, I also intend to move all the get_num_X_digits_in_Y
> > > and fill_string_with_X macros from the foreign_decl block at the start
> > > of the implementation section into a new file in the runtime, and
> > > replace them a #include of that new file.
> >
> > What's the benefit of putting them in the runtime? The only users of
> > those macros
> > are in the string module,
>
> Purely the reduction in the size of string.m. We *could* put the new header
> file in the library directory, but it would be the first one, and I don't see
> any point in duplicating the runtime directory's machinery for handling
> such files.
No, don't put header files in the library directory.
> > and under your proposal below would be moved to
> > string.from_builtin or string.to_builtin anyway.
>
> I described the two proposals independently, but they
> may have interactions (see below).
>
> > > I also intend to move the implementations of the builtin_type_to_string
> > > and string_to_builtin_type kinds of predicates (including the string to int
> > > predicates) to new submodules of string.m, named maybe string.from_builtin
> > > and string.to_builtin.
> >
> > My suggestion is:
> >
> > * Leave the character conversions where they are.
>
> Agreed.
>
> > * Move all of integer type conversion to string.from_int_type and
> > string.to_int_type.
> > * Move all of the float type conversions to string.from_float_type and
> > string.to_float_type
>
> The float conversions, and even some int conversions, are implemented
> directly using foreign_procs. While moving them to a submodule would be nice,
> this would be possibly by adding a forwarding predicate, and the associated
> performance loss would make that a bad idea. I intend to move to a new
> submodule only predicates that (a) already exist, and (b) are not exported
> to users.
>
> I think, but haven't checked, that all such predicates deal with ints/uints
I think that's the case.
> of various kinds. Therefore {from,to}_int_type would work as a name.
> (Though I would prefer just {from,to}_int.)
{from,to}_int are fine as well.
> Note that the int conversion foreign_procs are the ones that use
> the macros proposed to be moved to a new header file, so the
> #include probably won't have to move.
>
> > The reason I would make this split is that we haven't really addressed
> > conversion to / from floats in any meaningful way -- the C grades currently rely
> > on sprintf and sscanf -- but it is likely we shall do so one day and when
> > that occurs there will be rather more code for the float conversions present.
>
> The main question this raises is: when this happens, should that new code
> go in the same submodule(s) that handle int conversions, or into separate
> submodule(s)? Using a submodule name containing "builtin" would be
> appropriate for the former, names containing "int" likewise for the latter.
Such code is not going to be related to the int code, so there's no reason for
it to go in the same submodule.
Julien.
More information about the reviews
mailing list