| Message ID | 1430537453-25556-1-git-send-email-tbsaunde+gcc@tbsaunde.org |
|---|---|
| State | New |
| Headers | show |
> On May 1, 2015, at 8:30 PM, tbsaunde+gcc@tbsaunde.org wrote: > > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org> > > Hi, > > This adds a configure check to libobjc to find out if types of bitfields effect > their layout, and uses it to replace the rather broken usage of > PCC_BITFIELD_TYPE_MATTERS. > > bootstrapped + regtested x86_64-linux-gnu, bootstrapped on ppc64le-linux-gnu > and ran check-objc there without failures, and checked the correct part of the > ifdef is used on a cross to m68k-linux-elf. ok? I'm sure I've gotten > something wrong since this is a bunch of auto tools ;-) This is ok. I have been meaning to try to get rid of all uses of the target headers but never got around to finishing it. Thanks, Andrew > > Trev > > libobjc/ChangeLog: > > 2015-05-01 Trevor Saunders <tbsaunde+gcc@tbsaunde.org> > > * acinclude.m4: Include bitfields.m4. > * config.h.in: Regenerate. > * configure: Likewise. > * configure.ac: Invoke gt_BITFIELD_TYPE_MATTERS. > * encoding.c: Check HAVE_BITFIELD_TYPE_MATTERS. > > config/ChangeLog: > > 2015-05-01 Trevor Saunders <tbsaunde+gcc@tbsaunde.org> > > * bitfields.m4: New file. > --- > config/bitfields.m4 | 26 ++++++++++++++++++++++++++ > libobjc/acinclude.m4 | 1 + > libobjc/config.h.in | 3 +++ > libobjc/configure | 38 ++++++++++++++++++++++++++++++++++++++ > libobjc/configure.ac | 2 ++ > libobjc/encoding.c | 3 ++- > 6 files changed, 72 insertions(+), 1 deletion(-) > create mode 100644 config/bitfields.m4 > > diff --git a/config/bitfields.m4 b/config/bitfields.m4 > new file mode 100644 > index 0000000..ee8f3b5 > --- /dev/null > +++ b/config/bitfields.m4 > @@ -0,0 +1,26 @@ > +dnl Copyright (C) 2015 Free Software Foundation, Inc. > +dnl This file is free software, distributed under the terms of the GNU > +dnl General Public License. As a special exception to the GNU General > +dnl Public License, this file may be distributed as part of a program > +dnl that contains a configuration script generated by Autoconf, under > +dnl the same distribution terms as the rest of that program. > + > +# Define HAVE_BITFIELD_TYPE_MATTERS if the type of bitfields effects their > +# alignment. > + > +AC_DEFUN([gt_BITFIELD_TYPE_MATTERS], > +[ > + AC_CACHE_CHECK([if the type of bitfields matters], gt_cv_bitfield_type_matters, > + [ > + AC_TRY_COMPILE( > + [struct foo1 { char x; char :0; char y; }; > +struct foo2 { char x; int :0; char y; }; > +int foo1test[ sizeof (struct foo1) == 2 ? 1 : -1 ]; > +int foo2test[ sizeof (struct foo2) == 5 ? 1 : -1]; ], > + [], gt_cv_bitfield_type_matters=yes, gt_cv_bitfield_type_matters=no) > + ]) > + if test $gt_cv_bitfield_type_matters = yes; then > + AC_DEFINE(HAVE_BITFIELD_TYPE_MATTERS, 1, > + [Define if the type of bitfields effects alignment.]) > + fi > +]) > diff --git a/libobjc/acinclude.m4 b/libobjc/acinclude.m4 > index bf78dbe..4193018 100644 > --- a/libobjc/acinclude.m4 > +++ b/libobjc/acinclude.m4 > @@ -12,6 +12,7 @@ m4_include(../config/acx.m4) > m4_include(../config/no-executables.m4) > m4_include(../config/enable.m4) > m4_include(../config/tls.m4) > +m4_include(../config/bitfields.m4) > > m4_include(../libtool.m4) > dnl The lines below arrange for aclocal not to bring an installed > diff --git a/libobjc/config.h.in b/libobjc/config.h.in > index c055e7c..20d1fca 100644 > --- a/libobjc/config.h.in > +++ b/libobjc/config.h.in > @@ -1,5 +1,8 @@ > /* config.h.in. Generated from configure.ac by autoheader. */ > > +/* Define if the type of bitfields effects alignment. */ > +#undef HAVE_BITFIELD_TYPE_MATTERS > + > /* Define to 1 if the target assembler supports thread-local storage. */ > #undef HAVE_CC_TLS > > diff --git a/libobjc/configure b/libobjc/configure > index 642eb9c..0547f91 100755 > --- a/libobjc/configure > +++ b/libobjc/configure > @@ -11530,6 +11530,44 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu > { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_exception_model_name" >&5 > $as_echo "$ac_exception_model_name" >&6; } > > + > + { $as_echo "$as_me:${as_lineno-$LINENO}: checking if the type of bitfields matters" >&5 > +$as_echo_n "checking if the type of bitfields matters... " >&6; } > +if test "${gt_cv_bitfield_type_matters+set}" = set; then : > + $as_echo_n "(cached) " >&6 > +else > + > + cat confdefs.h - <<_ACEOF >conftest.$ac_ext > +/* end confdefs.h. */ > +struct foo1 { char x; char :0; char y; }; > +struct foo2 { char x; int :0; char y; }; > +int foo1test[ sizeof (struct foo1) == 2 ? 1 : -1 ]; > +int foo2test[ sizeof (struct foo2) == 5 ? 1 : -1]; > +int > +main () > +{ > + > + ; > + return 0; > +} > +_ACEOF > +if ac_fn_c_try_compile "$LINENO"; then : > + gt_cv_bitfield_type_matters=yes > +else > + gt_cv_bitfield_type_matters=no > +fi > +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext > + > +fi > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gt_cv_bitfield_type_matters" >&5 > +$as_echo "$gt_cv_bitfield_type_matters" >&6; } > + if test $gt_cv_bitfield_type_matters = yes; then > + > +$as_echo "#define HAVE_BITFIELD_TYPE_MATTERS 1" >>confdefs.h > + > + fi > + > + > # ------ > # Output > # ------ > diff --git a/libobjc/configure.ac b/libobjc/configure.ac > index c794a80..2d88519 100644 > --- a/libobjc/configure.ac > +++ b/libobjc/configure.ac > @@ -266,6 +266,8 @@ fi > AC_LANG_POP(C) > AC_MSG_RESULT($ac_exception_model_name) > > +gt_BITFIELD_TYPE_MATTERS > + > # ------ > # Output > # ------ > diff --git a/libobjc/encoding.c b/libobjc/encoding.c > index 20ace46..abb6145 100644 > --- a/libobjc/encoding.c > +++ b/libobjc/encoding.c > @@ -29,6 +29,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see > /* FIXME: This file contains functions that will abort the entire > program if they fail. Is that really needed ? */ > > +#include "config.h" > #include "objc-private/common.h" > #include "objc-private/error.h" > #include "tconfig.h" > @@ -1167,7 +1168,7 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout) > /* Record must have at least as much alignment as any field. > Otherwise, the alignment of the field within the record > is meaningless. */ > -#if !PCC_BITFIELD_TYPE_MATTERS > +#ifndef HAVE_BITFIELD_TYPE_MATTERS > layout->record_align = MAX (layout->record_align, desired_align); > #else /* PCC_BITFIELD_TYPE_MATTERS */ > if (*type == _C_BFLD) > -- > 2.3.0.80.g18d0fec.dirty >
On Fri, May 01, 2015 at 11:30:53PM -0400, tbsaunde+gcc@tbsaunde.org wrote: > + AC_TRY_COMPILE( > + [struct foo1 { char x; char :0; char y; }; > +struct foo2 { char x; int :0; char y; }; > +int foo1test[ sizeof (struct foo1) == 2 ? 1 : -1 ]; > +int foo2test[ sizeof (struct foo2) == 5 ? 1 : -1]; ], Shouldn't the 5 be sizeof (int) + 1? I mean, we have targets with 16-bit ints. I hope no target sizeof (int) == 1, that would break this test too (perhaps you could use long long :0; instead?). Also, the anon bitfield changes alignment only on a subset of targets: targetm.align_anon_bitfield () says if it makes a difference. So, wouldn't it be better to test instead if struct C { char a; char b : 1; char c; }; struct D { char a; long long b : 1; char c; }; int footest[sizeof (struct C) < sizeof (struct D)] ? 1 : -1]; ? Tested that it works with powerpc compiler with -mbit-align vs. -mno-bit-align. Jakub
On Sat, May 02, 2015 at 10:03:13AM +0200, Jakub Jelinek wrote: > On Fri, May 01, 2015 at 11:30:53PM -0400, tbsaunde+gcc@tbsaunde.org wrote: > > + AC_TRY_COMPILE( > > + [struct foo1 { char x; char :0; char y; }; > > +struct foo2 { char x; int :0; char y; }; > > +int foo1test[ sizeof (struct foo1) == 2 ? 1 : -1 ]; > > +int foo2test[ sizeof (struct foo2) == 5 ? 1 : -1]; ], > > Shouldn't the 5 be sizeof (int) + 1? I mean, we have targets with 16-bit > ints. I hope no target sizeof (int) == 1, that would break this test too > (perhaps you could use long long :0; instead?). yeah, I just mindlessly ported the test program in tm.texi. I'm dubious anyone tries to use objective C on machines with 16 bit int, and doing it on a machine with 8 bit int sounds insane, but who knows, and maybe something else will need this test some day? > Also, the anon bitfield changes alignment only on a subset of targets: > targetm.align_anon_bitfield () > says if it makes a difference. the use of anon bitfields with width 0 seemed dubious to me without knowing about this. > So, wouldn't it be better to test instead if > struct C { char a; char b : 1; char c; }; > struct D { char a; long long b : 1; char c; }; > int footest[sizeof (struct C) < sizeof (struct D)] ? 1 : -1]; > ? Tested that it works with powerpc compiler with -mbit-align vs. > -mno-bit-align. seems reasonable to me. fwiw I committed the original patch last night since Andrew ok'd it, but obviously we can improve it. We should probably update the test in tm.texi at the same time, or I guess better yet just refer to the m4 test there. Trev > > Jakub
On Sat, May 02, 2015 at 09:08:21AM -0400, Trevor Saunders wrote: > > > + AC_TRY_COMPILE( > > > + [struct foo1 { char x; char :0; char y; }; > > > +struct foo2 { char x; int :0; char y; }; > > > +int foo1test[ sizeof (struct foo1) == 2 ? 1 : -1 ]; > > > +int foo2test[ sizeof (struct foo2) == 5 ? 1 : -1]; ], > > > > Shouldn't the 5 be sizeof (int) + 1? I mean, we have targets with 16-bit > > ints. I hope no target sizeof (int) == 1, that would break this test too > > (perhaps you could use long long :0; instead?). > > yeah, I just mindlessly ported the test program in tm.texi. I'm dubious > anyone tries to use objective C on machines with 16 bit int, and doing > it on a machine with 8 bit int sounds insane, but who knows, and maybe > something else will need this test some day? 8-bit int isn't C compliant, generally, char has to be at least 8-bit, short and int at least 16-bit, long at least 32-bit and long long at least 64-bit (this comes from the C requirements of minimum value for U{CHAR,SHRT,INT,LONG,LLONG}_MAX). sizeof (int) == 1 was for the case where e.g. char would be 16-bit and int as well, or 32-bit char and 32-bit int, etc. Jakub
tbsaunde+gcc@tbsaunde.org writes: > +AC_DEFUN([gt_BITFIELD_TYPE_MATTERS], > +[ > + AC_CACHE_CHECK([if the type of bitfields matters], gt_cv_bitfield_type_matters, > + [ > + AC_TRY_COMPILE( > + [struct foo1 { char x; char :0; char y; }; > +struct foo2 { char x; int :0; char y; }; > +int foo1test[ sizeof (struct foo1) == 2 ? 1 : -1 ]; > +int foo2test[ sizeof (struct foo2) == 5 ? 1 : -1]; ], > + [], gt_cv_bitfield_type_matters=yes, gt_cv_bitfield_type_matters=no) > + ]) > + if test $gt_cv_bitfield_type_matters = yes; then > + AC_DEFINE(HAVE_BITFIELD_TYPE_MATTERS, 1, > + [Define if the type of bitfields effects alignment.]) > + fi > +]) gcc/config/aarch64/aarch64.h:#define PCC_BITFIELD_TYPE_MATTERS 1 configure:11554: /opt/gcc/gcc-20150503/Build/./gcc/xgcc -B/opt/gcc/gcc-20150503/Build/./gcc/ -B/usr/aarch64-suse-linux/bin/ -B/usr/aarch64-suse-linux/lib/ -isystem /usr/aarch64-suse-linux/include -isystem /usr/aarch64-suse-linux/sys-include -c -O2 -g conftest.c >&5 conftest.c:27:5: error: size of array 'foo2test' is negative int foo2test[ sizeof (struct foo2) == 5 ? 1 : -1]; ^ configure:11554: $? = 1 Andreas.
On Sun, May 03, 2015 at 10:59:46AM +0200, Andreas Schwab wrote: > tbsaunde+gcc@tbsaunde.org writes: > > > +AC_DEFUN([gt_BITFIELD_TYPE_MATTERS], > > +[ > > + AC_CACHE_CHECK([if the type of bitfields matters], gt_cv_bitfield_type_matters, > > + [ > > + AC_TRY_COMPILE( > > + [struct foo1 { char x; char :0; char y; }; > > +struct foo2 { char x; int :0; char y; }; > > +int foo1test[ sizeof (struct foo1) == 2 ? 1 : -1 ]; > > +int foo2test[ sizeof (struct foo2) == 5 ? 1 : -1]; ], > > + [], gt_cv_bitfield_type_matters=yes, gt_cv_bitfield_type_matters=no) > > + ]) > > + if test $gt_cv_bitfield_type_matters = yes; then > > + AC_DEFINE(HAVE_BITFIELD_TYPE_MATTERS, 1, > > + [Define if the type of bitfields effects alignment.]) > > + fi > > +]) > > gcc/config/aarch64/aarch64.h:#define PCC_BITFIELD_TYPE_MATTERS 1 > > configure:11554: /opt/gcc/gcc-20150503/Build/./gcc/xgcc -B/opt/gcc/gcc-20150503/Build/./gcc/ -B/usr/aarch64-suse-linux/bin/ -B/usr/aarch64-suse-linux/lib/ -isystem /usr/aarch64-suse-linux/include -isystem /usr/aarch64-suse-linux/sys-include -c -O2 -g conftest.c >&5 > conftest.c:27:5: error: size of array 'foo2test' is negative > int foo2test[ sizeof (struct foo2) == 5 ? 1 : -1]; > ^ > configure:11554: $? = 1 ok, a quick test seems to show Jakub's version of the test works in this case so lets try that. Trev > > Andreas. > > -- > Andreas Schwab, schwab@linux-m68k.org > GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 > "And now for something completely different."
On 05/01/2015 09:30 PM, tbsaunde+gcc@tbsaunde.org wrote: > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org> > > Hi, > > This adds a configure check to libobjc to find out if types of bitfields effect > their layout, and uses it to replace the rather broken usage of > PCC_BITFIELD_TYPE_MATTERS. > > bootstrapped + regtested x86_64-linux-gnu, bootstrapped on ppc64le-linux-gnu > and ran check-objc there without failures, and checked the correct part of the > ifdef is used on a cross to m68k-linux-elf. ok? I'm sure I've gotten > something wrong since this is a bunch of auto tools ;-) > > Trev > > libobjc/ChangeLog: > > 2015-05-01 Trevor Saunders <tbsaunde+gcc@tbsaunde.org> > > * acinclude.m4: Include bitfields.m4. > * config.h.in: Regenerate. > * configure: Likewise. > * configure.ac: Invoke gt_BITFIELD_TYPE_MATTERS. > * encoding.c: Check HAVE_BITFIELD_TYPE_MATTERS. OK with the general direction here. If Jakub's test is better, then go with it as a follow-up. jeff
diff --git a/config/bitfields.m4 b/config/bitfields.m4 new file mode 100644 index 0000000..ee8f3b5 --- /dev/null +++ b/config/bitfields.m4 @@ -0,0 +1,26 @@ +dnl Copyright (C) 2015 Free Software Foundation, Inc. +dnl This file is free software, distributed under the terms of the GNU +dnl General Public License. As a special exception to the GNU General +dnl Public License, this file may be distributed as part of a program +dnl that contains a configuration script generated by Autoconf, under +dnl the same distribution terms as the rest of that program. + +# Define HAVE_BITFIELD_TYPE_MATTERS if the type of bitfields effects their +# alignment. + +AC_DEFUN([gt_BITFIELD_TYPE_MATTERS], +[ + AC_CACHE_CHECK([if the type of bitfields matters], gt_cv_bitfield_type_matters, + [ + AC_TRY_COMPILE( + [struct foo1 { char x; char :0; char y; }; +struct foo2 { char x; int :0; char y; }; +int foo1test[ sizeof (struct foo1) == 2 ? 1 : -1 ]; +int foo2test[ sizeof (struct foo2) == 5 ? 1 : -1]; ], + [], gt_cv_bitfield_type_matters=yes, gt_cv_bitfield_type_matters=no) + ]) + if test $gt_cv_bitfield_type_matters = yes; then + AC_DEFINE(HAVE_BITFIELD_TYPE_MATTERS, 1, + [Define if the type of bitfields effects alignment.]) + fi +]) diff --git a/libobjc/acinclude.m4 b/libobjc/acinclude.m4 index bf78dbe..4193018 100644 --- a/libobjc/acinclude.m4 +++ b/libobjc/acinclude.m4 @@ -12,6 +12,7 @@ m4_include(../config/acx.m4) m4_include(../config/no-executables.m4) m4_include(../config/enable.m4) m4_include(../config/tls.m4) +m4_include(../config/bitfields.m4) m4_include(../libtool.m4) dnl The lines below arrange for aclocal not to bring an installed diff --git a/libobjc/config.h.in b/libobjc/config.h.in index c055e7c..20d1fca 100644 --- a/libobjc/config.h.in +++ b/libobjc/config.h.in @@ -1,5 +1,8 @@ /* config.h.in. Generated from configure.ac by autoheader. */ +/* Define if the type of bitfields effects alignment. */ +#undef HAVE_BITFIELD_TYPE_MATTERS + /* Define to 1 if the target assembler supports thread-local storage. */ #undef HAVE_CC_TLS diff --git a/libobjc/configure b/libobjc/configure index 642eb9c..0547f91 100755 --- a/libobjc/configure +++ b/libobjc/configure @@ -11530,6 +11530,44 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_exception_model_name" >&5 $as_echo "$ac_exception_model_name" >&6; } + + { $as_echo "$as_me:${as_lineno-$LINENO}: checking if the type of bitfields matters" >&5 +$as_echo_n "checking if the type of bitfields matters... " >&6; } +if test "${gt_cv_bitfield_type_matters+set}" = set; then : + $as_echo_n "(cached) " >&6 +else + + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +struct foo1 { char x; char :0; char y; }; +struct foo2 { char x; int :0; char y; }; +int foo1test[ sizeof (struct foo1) == 2 ? 1 : -1 ]; +int foo2test[ sizeof (struct foo2) == 5 ? 1 : -1]; +int +main () +{ + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO"; then : + gt_cv_bitfield_type_matters=yes +else + gt_cv_bitfield_type_matters=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext + +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gt_cv_bitfield_type_matters" >&5 +$as_echo "$gt_cv_bitfield_type_matters" >&6; } + if test $gt_cv_bitfield_type_matters = yes; then + +$as_echo "#define HAVE_BITFIELD_TYPE_MATTERS 1" >>confdefs.h + + fi + + # ------ # Output # ------ diff --git a/libobjc/configure.ac b/libobjc/configure.ac index c794a80..2d88519 100644 --- a/libobjc/configure.ac +++ b/libobjc/configure.ac @@ -266,6 +266,8 @@ fi AC_LANG_POP(C) AC_MSG_RESULT($ac_exception_model_name) +gt_BITFIELD_TYPE_MATTERS + # ------ # Output # ------ diff --git a/libobjc/encoding.c b/libobjc/encoding.c index 20ace46..abb6145 100644 --- a/libobjc/encoding.c +++ b/libobjc/encoding.c @@ -29,6 +29,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see /* FIXME: This file contains functions that will abort the entire program if they fail. Is that really needed ? */ +#include "config.h" #include "objc-private/common.h" #include "objc-private/error.h" #include "tconfig.h" @@ -1167,7 +1168,7 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout) /* Record must have at least as much alignment as any field. Otherwise, the alignment of the field within the record is meaningless. */ -#if !PCC_BITFIELD_TYPE_MATTERS +#ifndef HAVE_BITFIELD_TYPE_MATTERS layout->record_align = MAX (layout->record_align, desired_align); #else /* PCC_BITFIELD_TYPE_MATTERS */ if (*type == _C_BFLD)
From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org> Hi, This adds a configure check to libobjc to find out if types of bitfields effect their layout, and uses it to replace the rather broken usage of PCC_BITFIELD_TYPE_MATTERS. bootstrapped + regtested x86_64-linux-gnu, bootstrapped on ppc64le-linux-gnu and ran check-objc there without failures, and checked the correct part of the ifdef is used on a cross to m68k-linux-elf. ok? I'm sure I've gotten something wrong since this is a bunch of auto tools ;-) Trev libobjc/ChangeLog: 2015-05-01 Trevor Saunders <tbsaunde+gcc@tbsaunde.org> * acinclude.m4: Include bitfields.m4. * config.h.in: Regenerate. * configure: Likewise. * configure.ac: Invoke gt_BITFIELD_TYPE_MATTERS. * encoding.c: Check HAVE_BITFIELD_TYPE_MATTERS. config/ChangeLog: 2015-05-01 Trevor Saunders <tbsaunde+gcc@tbsaunde.org> * bitfields.m4: New file. --- config/bitfields.m4 | 26 ++++++++++++++++++++++++++ libobjc/acinclude.m4 | 1 + libobjc/config.h.in | 3 +++ libobjc/configure | 38 ++++++++++++++++++++++++++++++++++++++ libobjc/configure.ac | 2 ++ libobjc/encoding.c | 3 ++- 6 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 config/bitfields.m4