[m-rev.] for post-commit review: float NULL

Zoltan Somogyi zoltan.somogyi at runbox.com
Thu Jun 7 10:36:58 AEST 2018


For post-commit review by anyone.

After this diff, the hl.gc grade (the high level data MLDS C grade)
still has one test case failure in tests/hard_coded. It is pack_int32.m.
The root cause of that is a fundamental clash between (a) the fact that
for this Mercury type

:- type struct
    --->    struct(int32, int32, int32, int32).

we generate this C type:

struct pack_int32__struct_0_s {
  int32_t pack_int32__struct_0__F1;
  int32_t pack_int32__struct_0__F2;
  int32_t pack_int32__struct_0__F3;
  int32_t pack_int32__struct_0__F4;
};

and (b) that we fill in the structure using this code:

     Dynamic_5 = (struct pack_int32__struct_0_s *) MR_new_object(struct pack_int32__struct_0_s *, ((MR_Integer) 4 * sizeof(MR_Word)), NULL, NULL);
      MR_hl_field(MR_mktag(0), Dynamic_5, 0) = ((MR_Box) (MR_Word) (Var_26));
      MR_hl_field(MR_mktag(0), Dynamic_5, 1) = ((MR_Box) (MR_Word) (Var_28));
      MR_hl_field(MR_mktag(0), Dynamic_5, 2) = ((MR_Box) (MR_Word) (Var_30));
      MR_hl_field(MR_mktag(0), Dynamic_5, 3) = ((MR_Box) (MR_Word) (Var_32));

The right hand sides of those assignments, and the definition of MR_hl_field,
which is

#define MR_hl_field(t, p, i)        ((MR_Box *) MR_body((p), (t)))[i]

agree that the fields all have type MR_Box, but they don't. On my 64 bit laptop,
that means that each assignment writes 64 bits, to a location computed by assuming
that each field is 64 bits. Both halves of that are wrong.

The simpler fix is to make each field of the structure an MR_Box, but that makes
the structure bigger. The better fix would be to generalize MR_hl_field to non-MR_Box
field types, and to teach the compiler to use that generality when needed, as here.
Unfortunately, I am pretty sure it is a significantly bigger job.

Does anyone depend on the performance of the hl.gc grade?

Zoltan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DIFF.null
Type: application/octet-stream
Size: 822 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20180607/23710725/attachment.obj>


More information about the reviews mailing list