On Sun, Nov 22, 2009 at 12:32:03PM -0600, Jeff Epler wrote:
> work around a gcc bug in order to properly align hal_float_t
>
> As discovered first by micges, gcc did not give 8-byte alignment
> when hal_float_t was placed in unions or structures. This
> appears to be a gcc bug, which I have filed as
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42098
I think it's best to skip this bugfix in 2.3.x, and I wanted to record
the reasons here.
The bug has two faces. First, structure size mismatches between C and
C++. This affects programs which have C and C++ parts and which use a
structure with hal_float_t that happens to be unaligned. The only such
structure in emc is the private struct hal_pin_t, and none of the C++ in
our tree uses the relevant fields of hal_pin_t. Software outside emc
should not be using hal_pin_t. On the other hand, there's a simple
workaround for separately-shipped software to make its own structures
consistent: manually specify the alignment with __attribute__
((aligned(8))) in the declaration of the struct field of type
hal_float_t.
Second, it causes hal_float_t to have the wrong alignment. The impact
of this is low for several reasons:
* fld/fst are always going to be atomic on non-SMP systems except in
the very rare case that the operation crosses a page boundary and
there's a fault accessing the second page (e.g., because it was
swapped out, which is never the case for the HAL shared memory area).
Thus, the misalignment doesn't affect anybody using our kernel on
32-bit Ubuntu
* on x86_64 the field is properly aligned anyway (the default alignment
of double is 8, not 4), so this doesn't affect anybody using 64-bit
Ubuntu (and they're probably rare anyway)
* signals are always properly aligned thanks to the hal memory
allocation routines
* this leaves: parameters and the 'dummysig' field in hal_pin_t which
in few or no cases are being both read and written in a running
system
The reasons not to change it are:
* the change is an ABI change, since it modifies the layout of
hal_pin_t and potentially any structure containing hal_float_t
* this change will in general increase the amount of hal shared memory
used, so it might make some complex configuration no longer fit
The worst case is something like
struct {
hal_float_t *somepin1;
hal_float_t someparam1;
hal_float_t *somepin2;
hal_float_t someparam2;
}
which inflates by 8 bytes from 24 to 32 bytes, or 33%.
Jeff
> http://git.linuxcnc.org/?p=emc2.git;a=commitdiff;h=d48c422
>
> ---
> src/hal/hal.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/hal/hal.h b/src/hal/hal.h
> index e4b2c9c..7d25bb1 100644
> --- a/src/hal/hal.h
> +++ b/src/hal/hal.h
> @@ -284,7 +284,7 @@ typedef volatile __u32 hal_u32_t;
> typedef volatile __s32 hal_s32_t;
> typedef double real_t __attribute__((aligned(8)));
> typedef __u64 ireal_t __attribute__((aligned(8))); // integral type as wide
> as real_t / hal_float_t
> -typedef volatile double hal_float_t __attribute__((aligned(8)));
> +#define hal_float_t volatile real_t
>
> /***********************************************************************
> * "LOCKING" FUNCTIONS *
>
> ------------------------------------------------------------------------------
> Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
> trial. Simplify your report design, integration and deployment - and focus on
> what you do best, core application coding. Discover what's new with
> Crystal Reports now. http://p.sf.net/sfu/bobj-july
> http://cvsbook.red-bean.com/ - CVS Guide and mandatory reading.
> _____________________________________________
> Emc-commit mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/emc-commit
>
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Emc-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/emc-developers
