This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch ObjC/Darwin 1/3] Fix wrong code gen for m64 Objective-C


>>>> Index: gcc/objc/objc-gnu-runtime-abi-01.c
>>>> [...]
>>>> +#ifndef TARGET_64BIT
>>>> +#define TARGET_64BIT 0
>>>> +#endif
>
> One could just remove the explicit padder field, I suppose - but one  
> presumes that the original author added it with intent and that the  
> code was duly reviewed and approved before it was applied to the branch.

I was myself asking about the GNU runtime, where there has never been any
padder field, yet it all works perfectly well on 64-bit machines. :-)

Actually, some explicit padding was added for __LP64__ to the runtime structure
declarations, mostly because the runtime structure declaration is still exposed
in public headers and hence imported by all ObjC software (this will change in 4.7
when we drop the old API), so if you compile ObjC code with -Wpadded (to check your
own ObjC code) you'll get continuous warnings from the ObjC headers unless we have
that explicit padding (which does nothing other than stop warnings).  But, we didn't
need to add it to the compiler - the compiler adds it silently for its own structures.

I guess Apple may have added the padding field for a similar reason ?  They probably
also thought they may have a use for it at some point in the future (as they could use
it for something new without breaking binary compatibility), so gave it a field name
and kept it there ?

--

Anyhow, in the case of NeXT/Apple, I quickly checked their runtime code, and assuming
I am looking at the correct file (objc-runtime-new.h), I see --

typedef struct class_ro_t {
    uint32_t flags;
    uint32_t instanceStart;
    uint32_t instanceSize;
#ifdef __LP64__
    uint32_t reserved;
#endif

    const uint8_t * ivarLayout;
    
    const char * name;
    const method_list_t * baseMethods;
    const protocol_list_t * baseProtocols;
    const ivar_list_t * ivars;

    const uint8_t * weakIvarLayout;
    const struct objc_property_list *baseProperties;
} class_ro_t;

I think the optional padding field you are trying to add is the one marked by #ifdef __LP64__.
It should all work even if you don't add it, but if you want to add it, you need to check
if you are compiling for an LP64 machine, and if so, add the padding.  I imagine you could
maybe check if the __LP64__ #define is defined by the preprocessor ?  A cpp_ call would presumably
work ?  That would be identical to the code they use there. :-)

Else, something like

  if (TYPE_PRECISION (long_integer_type_node) == 64
      && POINTER_SIZE == 64
      && TYPE_PRECISION (integer_type_node) == 32)

(code lifted from cppbuiltin.c) may allow you to decide if you are in the __LP64__
case or not ?

Joseph will most likely be able to recommend the best way of doing this check.

Shall we do the same for the GNU runtime ?  Probably not.  We know the code without any explicit
padding fine works on 64-bit (both __LP64__ and no __LP64__) and I'm reluctant to make any changes
at this stage.  I would remove TARGET_64BIT from the GNU runtime file, and any padding or references
to it.

Thanks


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]