[m-rev.] for review: fix float field structure padding/packing problems

[Julien Fischer]

On Mon, 5 Dec 2011, Julien Fischer wrote:

> On Mon, 5 Dec 2011, Peter Wang wrote:
>
>> :- pred mlds_output_scalar_cell_group_type_and_name(mlds_to_c_opts::in,
>> diff --git a/runtime/mercury_float.h b/runtime/mercury_float.h
>> index 049477f..28d5728 100644
>> --- a/runtime/mercury_float.h
>> +++ b/runtime/mercury_float.h
>> @@ -29,6 +29,21 @@
>> #define MR_FLOAT_WORDS		((sizeof(MR_Float) + sizeof(MR_Word) 
>> - 1) \
>> 					/ sizeof(MR_Word))
>> 
>> +  /*
>> +  ** MR_Float_Aligned is a word-aligned version of MR_Float.  This is 
>> necessary
>> +  ** when MR_Float is single precision on a 64-bit platform to avoid 
>> packing
>> +  ** consecutive float fields, or when MR_Float is double precision on 
>> some
>> +  ** 32-bit platforms (e.g. Win32) to avoid introducing padding.
>> +  */
>> +#if defined(MR_GNUC) || defined(MR_CLANG)
>> +  typedef MR_Float MR_Float_Aligned 
>> __attribute__((aligned(sizeof(MR_Word))));
>> +#elif defined(MR_MSVC)
>> +  typedef _declspec(align(sizeof(MR_Word))) MR_Float MR_Float_Aligned;
>
> You didn't really think it would be that easy, did you? ;-)
>
> s/_declspec/__declspec/
>
> and apparently the argument of the align attribute must be an integer
> constant, so we can't use an expression such as sizeof(MR_Word).
> (This is with MSVC 2008, I haven't yet tried a later version.)
>
> I think the appropriate thing to do here is add MR_BYTES_PER_WORD back
> to the runtime and use that in the above.  (I say add back, because the
> only place it appears is in mercury_conf_bootstrap.h.)
>
> In the meantime I've just hardcoded a value in there - I'll let you know
> how that went once my workspace finishes building.

By itself the above does not work with MSVC.  We also need to pass the -Zp4
as a flag to cl, then the test case works.

Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------