[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