| Message ID | 1446205692-22412-5-git-send-email-tbsaunde+gcc@tbsaunde.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Oct 30, 2015 at 7:48 PM, <tbsaunde+gcc@tbsaunde.org> wrote: > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org> > > Similar to ROUND_TYPE_ALIGN it seems to make sense to copy the > information in the target macros to libobjc as an incremental step. Its > worth noting a large portion of the definitions of this macro only exist > to work around ADJUST_FIELD_ALIGN being used in target libs, so once all > target macros are gone from target libs we should be able to remove most > of the definitions of BIGGEST_FIELD_ALIGNMENT in gcc/, at which point > there won't be a significant amount of dupplication. > > libobjc/ChangeLog: > > 2015-10-30 Trevor Saunders <tbsaunde+gcc@tbsaunde.org> > > PR libobjc/24775 > * encoding.c (objc_layout_structure_next_member): Remove usage > of BIGGEST_FIELD_ALIGNMENT. > --- > libobjc/encoding.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/libobjc/encoding.c b/libobjc/encoding.c > index 867372d..7438d64 100644 > --- a/libobjc/encoding.c > +++ b/libobjc/encoding.c > @@ -1158,8 +1158,18 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout) > } > > /* The following won't work for vectors. */ > -#ifdef BIGGEST_FIELD_ALIGNMENT > - desired_align = MIN (desired_align, BIGGEST_FIELD_ALIGNMENT); > +#if defined __x86_64__ || defined __i386__ > +#if defined __CYGWIN__ || defined __MINGW32__ > + desired_align = MIN (desired_align, 64); > +#elif defined __x86_64__ > + desired_align = MIN (desired_align, 128); > +#else > + desired_align = MIN (desired_align, 32); > +#endif > +#elif defined __tilepro__ || defined __frv__ || defined __arm__ > + desired_align = MIN (desired_align, 64); > +#elif defined __tilegx__ > + desired_align = MIN (desired_align, 128); > #endif > #ifdef ADJUST_FIELD_ALIGN > desired_align = ADJUST_FIELD_ALIGN (type, desired_align); Just like the other patch, I would rather have a header files for each of the target instead so we separate out the target specific code from the target independent code. And so when porting to a new arch, it is easier to figure out all of the macros that need to be defined rather than having to audit the code. Thanks, Andrew > -- > 2.6.2 >
On Sat, Oct 31, 2015 at 01:20:45PM +0800, Andrew Pinski wrote: > On Fri, Oct 30, 2015 at 7:48 PM, <tbsaunde+gcc@tbsaunde.org> wrote: > > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org> > > > > Similar to ROUND_TYPE_ALIGN it seems to make sense to copy the > > information in the target macros to libobjc as an incremental step. Its > > worth noting a large portion of the definitions of this macro only exist > > to work around ADJUST_FIELD_ALIGN being used in target libs, so once all > > target macros are gone from target libs we should be able to remove most > > of the definitions of BIGGEST_FIELD_ALIGNMENT in gcc/, at which point > > there won't be a significant amount of dupplication. > > > > libobjc/ChangeLog: > > > > 2015-10-30 Trevor Saunders <tbsaunde+gcc@tbsaunde.org> > > > > PR libobjc/24775 > > * encoding.c (objc_layout_structure_next_member): Remove usage > > of BIGGEST_FIELD_ALIGNMENT. > > --- > > libobjc/encoding.c | 14 ++++++++++++-- > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/libobjc/encoding.c b/libobjc/encoding.c > > index 867372d..7438d64 100644 > > --- a/libobjc/encoding.c > > +++ b/libobjc/encoding.c > > @@ -1158,8 +1158,18 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout) > > } > > > > /* The following won't work for vectors. */ > > -#ifdef BIGGEST_FIELD_ALIGNMENT > > - desired_align = MIN (desired_align, BIGGEST_FIELD_ALIGNMENT); > > +#if defined __x86_64__ || defined __i386__ > > +#if defined __CYGWIN__ || defined __MINGW32__ > > + desired_align = MIN (desired_align, 64); > > +#elif defined __x86_64__ > > + desired_align = MIN (desired_align, 128); > > +#else > > + desired_align = MIN (desired_align, 32); > > +#endif > > +#elif defined __tilepro__ || defined __frv__ || defined __arm__ > > + desired_align = MIN (desired_align, 64); > > +#elif defined __tilegx__ > > + desired_align = MIN (desired_align, 128); > > #endif > > #ifdef ADJUST_FIELD_ALIGN > > desired_align = ADJUST_FIELD_ALIGN (type, desired_align); > > Just like the other patch, I would rather have a header files for each > of the target instead so we separate out the target specific code from > the target independent code. > And so when porting to a new arch, it is easier to figure out all of > the macros that need to be defined rather than having to audit the > code. Well ok that's an argument we can at least discuss :) Avoiding tempting people to dupplicate the same target specific code in multiple places is certainly of value, and making porting easier is also useful. On the other hand I doubt that many people care to port libojbc in particular to a new platform, and as I pointed out in another mail there's another libobjc out there from the gnustep people which I would guess people might be more likely to port? It also assumes someone would port the library by looking for macros to change rather than trying to build the library and then test that it works correctly, and if not debug the issues they find. As for avoiding dupplication stuff can easily be moved around by someone who actually wants to work on libobjc and is trying to add new things that would actually otherwise require the dupplication. Also moving the ifdef parts out into there own function, and probably even their own .c file isn't that much work. Supposing you wanted to move this code into macros in a config/ you'd also have to clean up the target dependent stuff at the top of the file that this code uses, and that sounds like more work than I'm really interested in doing right now. thanks Trev > > Thanks, > Andrew > > > -- > > 2.6.2 > >
diff --git a/libobjc/encoding.c b/libobjc/encoding.c index 867372d..7438d64 100644 --- a/libobjc/encoding.c +++ b/libobjc/encoding.c @@ -1158,8 +1158,18 @@ objc_layout_structure_next_member (struct objc_struct_layout *layout) } /* The following won't work for vectors. */ -#ifdef BIGGEST_FIELD_ALIGNMENT - desired_align = MIN (desired_align, BIGGEST_FIELD_ALIGNMENT); +#if defined __x86_64__ || defined __i386__ +#if defined __CYGWIN__ || defined __MINGW32__ + desired_align = MIN (desired_align, 64); +#elif defined __x86_64__ + desired_align = MIN (desired_align, 128); +#else + desired_align = MIN (desired_align, 32); +#endif +#elif defined __tilepro__ || defined __frv__ || defined __arm__ + desired_align = MIN (desired_align, 64); +#elif defined __tilegx__ + desired_align = MIN (desired_align, 128); #endif #ifdef ADJUST_FIELD_ALIGN desired_align = ADJUST_FIELD_ALIGN (type, desired_align);
From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org> Similar to ROUND_TYPE_ALIGN it seems to make sense to copy the information in the target macros to libobjc as an incremental step. Its worth noting a large portion of the definitions of this macro only exist to work around ADJUST_FIELD_ALIGN being used in target libs, so once all target macros are gone from target libs we should be able to remove most of the definitions of BIGGEST_FIELD_ALIGNMENT in gcc/, at which point there won't be a significant amount of dupplication. libobjc/ChangeLog: 2015-10-30 Trevor Saunders <tbsaunde+gcc@tbsaunde.org> PR libobjc/24775 * encoding.c (objc_layout_structure_next_member): Remove usage of BIGGEST_FIELD_ALIGNMENT. --- libobjc/encoding.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)