[m-rev.] for review: replace four "safe num digits" tables with one
Zoltan Somogyi
zoltan.somogyi at runbox.com
Tue Sep 15 23:33:58 AEST 2026
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.
The compiler does keep the big table predicate around, because string.m
has submodules, and it does not know that the submodules do not refer
to it either. Moving that code to a submodule would fix that.
> > 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.
> 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
of various kinds. Therefore {from,to}_int_type would work as a name.
(Though I would prefer just {from,to}_int.)
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.
Like you seem to, I favor the type-specific submodule option.
> > (string.to_string is already taken.)
>
> It's a private submodule and could be untaken, although I think its current
> name is fine.
Exactly why said what I did.
> > The objective of all this is to reduce the size of string.m (which is now
> > the largest library module by far) and keep it manageable.
>
> The same though occurred to me the other day when I was working on it,
> so no objection from me.
Thanks.
Zoltan.
More information about the reviews
mailing list