On Fri, Oct 30, 2015 at 01:16:16PM +0100, Richard Biener wrote: > On Fri, Oct 30, 2015 at 1:06 PM, Bernd Schmidt <bschm...@redhat.com> wrote: > > On 10/30/2015 12:48 PM, tbsaunde+...@tbsaunde.org wrote: > >> > >> -#ifdef ADJUST_FIELD_ALIGN > >> - desired_align = ADJUST_FIELD_ALIGN (type, desired_align); > >> +#if defined __arc__ || defined _AIX > >> + if (TYPE_MODE (strip_array_types (TREE_TYPE (type))) == DFmode) > >> + desired_align = MIN (desired_align, 32); > >> +#elif __POWERPC__ && __APPLE__ > >> + if (desired_align != 128) > >> + desired_align = MIN (desired_align, 32); > >> #endif > > > > > > No way. We never use this kind of test in target-independent code. > > it's not target independent code. Are you suggesting to add a config/ > to libobjc? IMHO for a not really mantained frontend / target lib that's > an excessive requirement.
Given the amount of target dependant stuff involved adding a config/ actually seems worse to me. You are accomplishing the exact same thing, but you need a whole lot more machinary to do it, and its hard to understand what happens for any given platform. Sure, if there was a whole lot more target code doing something else might make sense, but there isn't and I'm certainly not planning on adding more. SO I think its best to leave it this way and if someone wants to do substantial work on libobjc in the future they can worry about that then. btw the claim its never done just doesn't stand up either, look at the __SPARC__ check this series removes, or the __MINGW__ check in gthr.h, or even all the crap at the top of encoding.c that makes using these target macros possible (it wouldn't actually suprise me if cleaning that up ment doing this was a net reduction in target dependent code in encoding.c). If you want to be kind of sad I discovered https://github.com/gnustep/libobjc2 while looking at this, so it seems like many of the possible users of libobjc may even be not using it. > For any such replacements as in the patch I suggest to at least keep a comment > before it indicating the origin of the inlined vairants (in this case refer to > ADJUST_FIELD_ALIGN). That seems fairly reasonable, I'd kind of worry about them getting out of date, but i guess it at least gives a place to start looking for an explanation. > In general I'm happy with this kind of patches (maybe not the > BIGGEST_FIELD_ALIGN > one which could be made a CPP macro when -fbuilding-libgcc) I considered that, but the only targets that define BIGGEST_FIELD_ALIGNMENT for purposes other than IN_TARGET_LIBS hacks were v850, vax, tilegx, and tilepro so considering BIGGEST_FIELD_ALIGNMENT kind of dupplicates ADJUST_FIELD_ALIGN my conclusion was it would make more sense to not do that. I'm thinking it makes sense to instead just merge BIGGEST_FIELD_ALIGNMENT into ADJUST_FIELD_ALIGN, but adding a predefined macro would make that harder. Trev > > Richard. > > > > > Bernd
