[m-rev.] for review: Fix inconsistency in type parameter layout structures.

[Zoltan Somogyi]

On Wed, 5 Jun 2019 21:56:10 +1000, Peter Wang <novalazy at gmail.com> wrote:

> On Wed, 05 Jun 2019 12:29:39 +0200 (CEST), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> > 
> > 
> > On Wed,  5 Jun 2019 18:02:20 +1000, Peter Wang <novalazy at gmail.com> wrote:
> > > Some help with the log message would be appreciated. I don't know if
> > > I've named the things correctly.
> > 
> > I am the person you need, since I wrote that code.
> > 
> > > The MR_TypeParamLocns_Struct field MR_tp_param_count has type
> > > MR_uint_least32_t, but the compiler generates type var vectors (???)
> > > in which every value is word sized.
> > 
> > I am not sure which vector you are talking about; there are so many
> > in the debugger RTTI. Is it the MR_tp_param_locns field that immediately
> > follows the count? If so, that doesn't hold type vars; it holds descriptions
> > of the locations in which the debugger can find the typeinfos describing
> > the types that are *bound* to those type vars.
> 
> The type parameters vector in the closure layout (I've disabled
> use_common_cells to make it clearer):
> 
>     static const MR_Integer typevar_vector_2[2] = {
>       (MR_Integer) 1,
>       (MR_Integer) 33
>     };
> 
>     static const MR_Box closure_layout_3[8] = {
>       NULL,
>       ((MR_Box) (&typevar_vector_2)),
>       ((MR_Box) ((MR_Integer) 5)),
>       ((MR_Box) (&mercury__private_builtin__private_builtin__type_ctor_info_type_info_0)),
>       ((MR_Box) ((MR_Integer) 1)),
>       ((MR_Box) ((MR_Integer) 1)),
>       ((MR_Box) (&mercury__builtin__builtin__type_ctor_info_string_0)),
>       ((MR_Box) (&mercury__builtin__builtin__type_ctor_info_string_0))
>     };

I see. Your diagnosis is correct. Basically the issue is: the compiler generates
a single vector that contains both the count and the location descriptions,
so their types must have the same size, but without this change, they don't.

> > > On 32-bit or little endian 64-bit
> > > machines the value will happen to be correct, but on big endian machines
> > > the value will be zero.
> > > 
> > > This bug caused hard_coded/copy_pred_2.m to fail on a 64-bit big endian
> > > machine (AIX/PowerPC).
> > > 
> > > runtime/mercury_stack_layout.h:
> > >     Change type of MR_tp_param_count to MR_Integer.
> > > 
> > > diff --git a/runtime/mercury_stack_layout.h b/runtime/mercury_stack_layout.h
> > > index 3e26e4877..627d30fbd 100644
> > > --- a/runtime/mercury_stack_layout.h
> > > +++ b/runtime/mercury_stack_layout.h
> > > @@ -476,7 +476,7 @@ struct MR_UserEventSpec_Struct {
> > >  #define MR_NOT_HIDDEN   0
> > >  
> > >  struct MR_TypeParamLocns_Struct {
> > > -    MR_uint_least32_t       MR_tp_param_count;
> > > +    MR_Integer              MR_tp_param_count;
> > >      MR_LongLval             MR_tp_param_locns[MR_VARIABLE_SIZED];
> > >  };
> > 
> > I think the right change is to MR_Unsigned, not MR_Integer.
> 
> The values are currently always assigned to local variables of type
> MR_Integer, so I will leave it as MR_Integer.

OK, but put an XXX next to it. It seems the type declarations (e.g. MR_LongLval)
use unsigned values, but neither the code in the runtime system (in trace/*.c as well
as in runtime/*.[ch]) nor the code that generates the definitions of static values
of those types have yet been updated to use unsigned values.

> (ok, having looked at that again, I think I can write a better log
> message tomorrow)

If you want, I can write both the XXX and the log message, and commit
the diff.

Zoltan.