[m-rev.] for review: pack small args next to local secondary tags

[Julien Fischer]

Hi Zoltan,

On Mon, 2 Jul 2018, Zoltan Somogyi wrote:

> For review by anyone. Note that so far, I have bootchecked it
> only in asm_fast.gc and hlc.gc with the new data representation
> enabled, but I will test it in other grades and with the new representation
> disabled before commit.

There are few tests cases (on my system) related to fixed size integers
currently failing in decldebug grades.  (One of which is the argument
packing test.)  (I've been meaning to investigate, but other things
keep getting in the way.)

> The reason why there are no new test cases is that the compiler,
> library and the existing test cases have several types that the
> new representation applies to. (I discovered this the hard way.)
> However, if anyone wants to propose new test cases, please
> feel free to ask for them.
>
> This is one of the data representation optimizations we have discussed
> before. Before I start on the next, packing small args next to *remote*
> secondary tags, or even extending this one to types with only one
> function symbol (see the Log file), I plan to make two kinds of
> housekeeping changes. One is to split up unify_gen.m and ml_unify_gen.m
> into several modules each, with the new modules implementing tests,
> constructions, deconstructions, and utility predicates respectively.
> The other is to remove unnecessary differences between the LLDS
> and MLDS versions of code generation for unifications. Some of
> the differences are of course needed, but much of the code now deals
> not with code generation itself but with the management of argument
> packing, so removing unnecessary differences seems a prerequisite
> to factoring out any commonalities between the backends.
>
> The attached diff would have been easier to write if I had made these
> two housekeeping changes before I started, but I discovered the need
> for them only while writing the diff. Only hindsight is 20/20 :-(
>
> Does anyone object to the module splits? After this diff, the two modules
> are about 4000 and 3000 lines respectively.

No objection from me.

> Pack subword-sized arguments next to a local sectag.
> 
> compiler/du_type_layout.m:
>     If a new option is set, then try to represent function symbols with
>     only subword-sized arguments by packing those arguments into the same word
>     as the primary tag and (if it is needed) a secondary tag.
>
>     If there are too many such function symbols for the available number of
>     bits, pick the ones that need the least number of bits, in order to
>     allow us to use this representation for as many such function symbols
>     as possible.
>
>     This diff implements this packing only for types that have more than one
>     argument, because implementing it for types that have only one argument
>     has two extra complications. One is the need for another new cons_id
>     (see below), which would make this diff bigger and harder to review.
>     The other is the need to consider interactions with the direct_arg
>     optimization.
>
>     Don't invoke the code for deciding the representation of arguments
>     if either (a) the function symbol has no arguments, or (b) its cons_id
>     alone dictates how we will treat its argument (in such cases, there is
>     always exactly one).
>
>     Fix a bug in computing the number of bits needed to distinguish N things.
>
>     Store the value of the experiment option in the params for now,

s/experiment/experimental/

...

> runtime/mercury_type_info.h:
>     Extend the representation of du functors in the RTTI to account for
>     the new data representation scheme. The extensions add only to the
>     *ends* of structures, or to lists of enum values, with the extensions
>     only being used if the representation is actually used, which should
>     allow the updated runtime to also work with .c files that were compiled
>     with a compiler that does *not* have this diff. For the same reason,
>     make the old enum value MR_SECTAG_LOCAL a symonym for the new

s/symonym/synonym/

>     MR_SECTAG_LOCAL_REST_OF_WORD, which expresses a distinction that
>     did not previously exist.


You (presumably) also need to update the C# definition of that enumeration;
in runtime/mercury_dotnet.cs.in.

library/rtti_implementation.m probably also requires updating.

...


> diff --git a/compiler/prog_data.m b/compiler/prog_data.m
> index a3459b9..681859e 100644
> --- a/compiler/prog_data.m
> +++ b/compiler/prog_data.m
> @@ -439,9 +439,12 @@ cons_id_is_const_struct(ConsId, ConstNum) :-
>              ).
>
>      % The arg_pos_width type and its components specify how much space
> -    % does a constructor argument occupy in the memory cell that
> -    % represents a term with that constructor, and where.
> +    % does a constructor argument occupy in the memory that represents
> +    % a term with that constructor, and where. This memory will usually be
> +    % in a heap cell, so this is what the discussion below assumes,
> +    % but see below for an exception.
>      %
> +    % XXX ARG_PACK document the CellOffset fields.
>      % `apw_full(ArgOnlyOffset)' indicates that the argument fully occupies
>      % a single word, and this word is ArgOnlyOffset words after the first word
>      % of the memory cell cell that starts storing visible arguments.
> @@ -484,16 +487,29 @@ cons_id_is_const_struct(ConsId, ConstNum) :-
>      % argument is missing, or if either is full word sized or larger,
>      % then the arguments in the run will all be apw_none_nowhere.
>      %
> -    % The EBNF grammar of possible sequences of argument representations is:
> -    %
> -    % constructor
> -    %   :  integral_word_unit*
> -    %
> -    % integral_word_unit
> -    %   :  apw_none_nowhere
> -    %   |  apw_full
> -    %   |  apw_double
> -    %   |  apw_partial_first (apw_none_shifted* apw_partial_shifted)+
> +    % The exception mentioned above is that if a function symbols only has

s/symbols/symbol/

> +    % a small number of small (subword-sized) arguments, then we try to fit
> +    % the representation of all the arguments next to the primary and local
> +    % secondary tags, *without* using a heap cell. In this case, all these
> +    % arguments will be represented by apw_partial_shifted with -1 as the
> +    % offset (both kinds), unless they are of a dummy type, in which case
> +    % their representation will be apw_none_shifted, also with -1 as offset.

That looks fine otherwise.

Julien.