[m-rev.] for review: fix float field structure padding/packing problems
Paul Bone
pbone at csse.unimelb.edu.au
Tue Dec 6 11:48:26 AEDT 2011
On Mon, Dec 05, 2011 at 05:50:36PM +1100, Peter Wang wrote:
> Can someone apply this and test with MSVC and MinGW?
> I've only tested the single precision floats on 64-bit Linux part.
>
> Would it be worth avoiding the C extensions when
> sizeof(MR_Float) == sizeof(MR_Word)? That can't be evaluated by the C
> preprocessor, though.
>
If this is possible then there's an improvement below (see my in-line reply)
that can be made.
> 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;
> +#else
> + /* Hope for the best. */
> + typedef MR_Float MR_Float_Aligned;
> +#endif
> +
This last alternative could raise an error to tell the user that we don't
support his/her compiler on this combination of word size and float size.
The rest is fine with me.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20111206/acaba3b5/attachment.sig>
More information about the reviews
mailing list