[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