[m-rev.] for review: Make subtypes share low-level data representation with base type.

[Julien Fischer]

Hi Peter,

On Thu, 25 Mar 2021, Peter Wang wrote:

> Make subtypes share data representation with base type when using
> low-level data. High-level data grades are unchanged, so subtypes
> are still represented with distinct classes from their base types.

...

> runtime/mercury_type_info.h:
>    Add a flag in MR_TypeCtorInfo to indicate if the enum/du layout
>    array may be indexed by an enum value or du ptag value.
>    Subtypes break the invariant that the layout array contains entries
>    for every enum/ptag value from 0 up to the maximum value.
>    The presence of the flag MR_TYPE_CTOR_FLAG_LAYOUT_INDEXABLE tells
>    the runtime that it can directly index the layout array instead of
>    searching through it (which is the common case, for non-subtypes).
>
>    Add a field MR_du_ptag to MR_DuPtagLayout. This is necessary to find
>    an entry for a given primary tag value in a MR_DuPtagLayout array.
>
>    Add a field MR_du_ptag_flags to MR_DuPtagLayout, currently with one
>    possible flag MR_DU_PTAG_FLAG_SECTAG_ALTERATIVES_INDEXABLE.

s/ALTERATIVES/ALTERNATIVES/  (and also in the code).

>    As with primary tags, subtypes break the invariant that the
>    sectag_alternatives array contains entries for every secondary tag
>    value from 0 up to the maximum value. The presence of the flag tells
>    the runtime that it can directly index the sectag_alternatives array
>    (which is the common case, for non-subtypes).
>
>    The two fields added to MR_DuPtagLayout occupy space that was
>    previously padding, so the size of MR_DuPtagLayout is unchanged.
>
>    In MR_EnumFunctorDesc, replace the MR_enum_functor_ordinal field by
>    MR_enum_functor_value, i.e. the integer value representing the
>    functor in memory. Storing *both* the functor ordinal and enum value
>    would increase the size of the MR_EnumFunctorDesc struct, and would
>    be redundant in the common case of non-subtype enums (both fields
>    would contain equal values).

IIRC, it wasn't redundant in the past when we supported reserve tag
pragmas / grades, but they're long since gone.

>    We forgo having the functor ordinal directly available, at the cost of
>    needing to search through an MR_EnumFunctorDesc array when a functor
>    ordinal is required for a subtype enum, which should be rare.

That should be fine; that's what we do for for foreign enums anyway.

> compiler/rtti.m:
>    Swap enum "functor ordinal" and "value" in places.
>
>    Use a type 'enum_value' to try to ensure we do not mix up enum
>    functor ordinals and enum values.
>
>    Add code to encode the MR_TYPE_CTOR_FLAG_LAYOUT_INDEXABLE flag.
>
>    Add code to encode the MR_DU_PTAG_FLAG_SECTAG_ALTERATIVES_INDEXABLE
>    flag.

ALTERNATIVES again.

> compiler/rtti_out.m:
>    Write out "enum_ordinal_ordered_tables" ordered by functor ordinals
>    instead of "enum_value_ordered_tables" ordered by enum values.
>
>    Output the enum value for MR_EnumFunctorDesc instead of functor
>    ordinal.
>
>    Output the MR_du_ptag and MR_du_ptag_flags fields for
>    MR_DuPtagLayout.
>
>    Relax sanity check on primary tags. A subtype may not necessarily
>    use ptag 0, and may skip ptag values.
>
> compiler/rtti_to_mlds.m:
>    Generate "enum_ordinal_ordered_tables" instead of
>    "enum_value_ordered_tables".
>
>    Fill in the enum value for a MR_EnumFunctorDesc instead of
>    the functor ordinal.
>
> compiler/type_ctor_info.m:
>    Add predicate to generate the MR_du_ptag_flags field.
>
>    Add the MR_TYPE_CTOR_FLAG_LAYOUT_INDEXABLE flag to type_ctor_infos
>    when appropriate.
>
>    Bump the type_ctor_info_rtti_version.

...

> diff --git a/compiler/check_parse_tree_type_defns.m b/compiler/check_parse_tree_type_defns.m
> index 38e5e2c16..33cc4638b 100644
> --- a/compiler/check_parse_tree_type_defns.m
> +++ b/compiler/check_parse_tree_type_defns.m
> diff --git a/compiler/comp_unit_interface.m b/compiler/comp_unit_interface.m
> index faf240fc2..8274b3b6f 100644
> --- a/compiler/comp_unit_interface.m
> +++ b/compiler/comp_unit_interface.m
> @@ -1,7 +1,7 @@
> %---------------------------------------------------------------------------%
> % vim: ft=mercury ts=4 sw=4 et
> %---------------------------------------------------------------------------%
> -% Copyright (C) 2015 The Mercury team.
> +% Copyright (C) 2015-2016, 2018-2021 The Mercury team.
> % This file may only be copied under the terms of the GNU General
> % Public License - see the file COPYING in the Mercury distribution.
> %---------------------------------------------------------------------------%
> @@ -129,6 +129,7 @@
> :- import_module parse_tree.prog_foreign.
> :- import_module parse_tree.prog_mutable.
> :- import_module parse_tree.prog_type.
> +:- import_module parse_tree.prog_type_subst.
> 
> :- import_module bool.
> :- import_module cord.
> @@ -1115,16 +1116,25 @@ accumulate_modules_from_type(Type, !Modules) :-
>     %   XXX TYPE_REPN Shouldn't boxing make the size of the foreign type
>     %   immaterial?
>     %
> -    % - If the definition defines an enum type, and there is a definition
> -    %   of the same type_ctor in the interface, we include the type_ctor in
> -    %   AbsExpEnumTypeCtors. This is so that when we abstract export
> -    %   the type_ctor, we can record that its size is less than one word.
> +    % - If the definition defines a subtype, and there is any definitions of

s/is any/are any/

...

> diff --git a/compiler/decide_type_repn.m b/compiler/decide_type_repn.m
> index b21828251..6a1a8e90c 100644
> --- a/compiler/decide_type_repn.m
> +++ b/compiler/decide_type_repn.m

> @@ -393,8 +453,13 @@ decide_type_repns_stage_1_du_not_all_plain_constants(TypeCtor, DuDefn,
>             )
>         ;
>             TailCtors = [_ | _]
> -        % The type has exactly two or more data constructors.
> +            % The type has two or more data constructors.
>             % This means that it need not be word aligned.

Has the indentation gone funny there or is that just a diff artifact?

> +        )
> +    ;
> +        MaybeSuperType = yes(_)
> +        % We cannot decide the representation of a subtype independently
> +        % of its base type, which may not even be in the same module.
>     ).
>

...

> diff --git a/runtime/mercury_type_info.h b/runtime/mercury_type_info.h
> index 796c62c8d..ed00dca4a 100644
> --- a/runtime/mercury_type_info.h
> +++ b/runtime/mercury_type_info.h

...

> @@ -1082,8 +1092,25 @@ typedef struct {
>     // compute it potentially millions of times. It could be stored either as
>     // a MR_uint_least32_t or as a MR_uint_least16_t, depending on how
>     // conservative we want to be.
> +
> +    // ptag holds the primary tag that this layout describes.
> +    // ptag_flags contains the flags listed below.
> +    // XXX ARG_PACK Move these fields at the same time as other fields.
> +    MR_uint_least8_t		    MR_du_ptag;
> +    MR_uint_least8_t		    MR_du_ptag_flags;
> } MR_DuPtagLayout;
> 
> +// The flag bits here must agree with the ones in encode_du_ptag_layout_flag
> +// in compiler/rtti.m, and with constants in java/runtime and
> +// mercury_dotnet.cs.in.
> +//
> +// The sectag_indexable flag is set if the sectag_alternatives array
> +// can be indexed using a secondary tag value.
> +#define MR_DU_PTAG_FLAG_SECTAG_ALTERATIVES_INDEXABLE	0x1

ALTERNATIVES again.

> +// An array of ptag layouts, ordered by ptag value.
> +// Whether or not it is indexable by ptag value is indicated by the
> +// MR_TYPE_CTOR_FLAG_LAYOUT_INDEXABLE flag.
> typedef const MR_DuPtagLayout *MR_DuTypeLayout;
> 
> ////////////////////////////////////////////////////////////////////////////

That looks fine otherwise.

Julien.