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

[Zoltan Somogyi]

On Tue, 3 Jul 2018 20:51:29 -0400 (EDT), Julien Fischer <jfischer at opturion.com> wrote:
> > 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.

The extra bootchecks revealed a problem, which was caused, in a very
roundabout way, by incorrect handling of RTTI-based term construction
of terms involving the new packing scheme. I therefore added a direct test
of this use case.

> > Does anyone object to the module splits? After this diff, the two modules
> > are about 4000 and 3000 lines respectively.
> 
> No objection from me.

I will do that after committing this diff.

> >     Store the value of the experiment option in the params for now,
> 
> s/experiment/experimental/

The name of the option is "experiment"; I added quotes to the log message.

> > For the same reason,
> >     make the old enum value MR_SECTAG_LOCAL a symonym for the new
> 
> s/symonym/synonym/

Done.

> >     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.

Done. You may want to have a look at the diffs of those files in the
attached updated diff. I updated anything that looked like it needed
updating, but I don't know whether the affected code is even used
currently, or just there are as a placeholder.

Nevertheless, I think problems with the Java and C# grades are unlikely, since
(a) the new packing scheme applies only to the low level data representation,
and (b) the updated compiler generates exactly the same Java and C# code as
the original compiler, modulo the extra field in functor descriptors, and these
are not needed if the new packing is not enabled.

> > +    % The exception mentioned above is that if a function symbols only has
> 
> s/symbols/symbol/

Done.

> > +    % 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.

Thanks for the review.

Zoltan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DIFF.lpack2
Type: application/octet-stream
Size: 225035 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20180708/17f6cffb/attachment-0001.obj>