[m-rev.] for review: tightening mlds_type

[Julien Fischer]

Hi Zoltan,

On Thu, 27 Sep 2018, Zoltan Somogyi wrote:

> We discussed this a few weeks ago. For review by anyone.

> Tigthen the mlds_type type.

s/Tigthen/Tighten/

> 
> compiler/mlds.m:
>     Make two changes to mlds_type.
>
>     The simpler change is the deletion of the maybe(foreign_type_assertions)
>     field from the MLDS representations of Mercury types. It was never used,
>     because Mercury types that are defined in a foreign language that is
>     acceptable for the current MLDS target platform are represented
>     as mlds_foreign_type, not as mercury_type.
>
>     The more involved change is to change the representation of builtin types.
>     Until now, we had separate function symbols in mlds_type to represent
>     ints, uints, floats and chars, but not strings or values of the sized
>     types {int,uint}{8,16,32,64}; those had to be represented as Mercury types.
>     This is an unnecessary inconsistency. It also had two allowed
>     representations for ints, uints, floats and chars, which meant that
>     some of the code handling those conceptual types had to be duplicated
>     to handle both representations.
>
>     This diff provides mlds_builtin_type_{int(_),float,string,char} function
>     symbols to represent every builtin type, and changes mercury_type
>     to mercury_nb_type to make clear that it is NOT to be used for builtins
>     (the nb is short for "not builtin").
> 
> compiler/ml_code_util.m:
> compiler/ml_util.m:
>     Delete functions that used to construct MLDS representations of builtin
>     types. The new representation of those types is so simple that using
>     such functions is no less cumbersome than writing down the representations
>     directly.

...

> diff --git a/compiler/ml_lookup_switch.m b/compiler/ml_lookup_switch.m
> index 5f88c95..9967140 100644
> --- a/compiler/ml_lookup_switch.m
> +++ b/compiler/ml_lookup_switch.m

...


> @@ -822,22 +820,39 @@ make_dummy_first_soln_row(FirstSolnStructType, FieldTypes,
>
>  ml_default_value_for_type(MLDS_Type) = DefaultRval :-
>      (
> -        MLDS_Type = mlds_native_int_type,
> -        DefaultRval = ml_const(mlconst_int(0))
> +        MLDS_Type = mlds_builtin_type_int(IntType),
> +        (
> +            ( IntType = int_type_int
> +            ; IntType = int_type_int8
> +            ; IntType = int_type_int16
> +            ; IntType = int_type_int32
> +            ; IntType = int_type_int64
> +            ; IntType = int_type_uint8
> +            ; IntType = int_type_uint16
> +            ; IntType = int_type_uint32
> +            ; IntType = int_type_uint64
> +            ),
> +            % XXX This is wrong in signedness and/or size
> +            % for all of the above except int_type_int.
> +            DefaultRval = ml_const(mlconst_int(0))

Is there a reason not to use the correct values now?

...

> diff --git a/compiler/options.m b/compiler/options.m
> index 03aed55..6768e6f 100644
> --- a/compiler/options.m
> +++ b/compiler/options.m
> @@ -1456,11 +1456,11 @@ option_defaults_2(compilation_model_option, [
>                                          % all word bits for argument packing.
>      allow_double_word_fields            -   bool(yes),
>      allow_double_word_ints              -   bool(no),
> -    allow_packing_dummies               -   bool(no),
> -    allow_packing_ints                  -   bool(no),
> -    allow_packing_chars                 -   bool(no),
> -    allow_packing_local_sectags         -   bool(no),
> -    allow_packing_remote_sectags        -   bool(no),
> +    allow_packing_dummies               -   bool(yes),
> +    allow_packing_ints                  -   bool(yes),
> +    allow_packing_chars                 -   bool(yes),
> +    allow_packing_local_sectags         -   bool(yes),
> +    allow_packing_remote_sectags        -   bool(yes),
>      allow_packing_mini_types            -   bool(no),
>      sync_term_size                      -   int(8),
>                                          % 8 is the size on linux (at the time

Did you mean to enable these now?

The rest looks fine.

Julien.