[PATCH v3] score: PR1607: Add and use CPU_SIZEOF_POINTER

[Ralf Corsepius]

On 11/08/2012 10:52 AM, Sebastian Huber wrote:
> On 11/08/2012 10:33 AM, Ralf Corsepius wrote:
>> On 11/08/2012 09:27 AM, Sebastian Huber wrote:
>>> Add and use new CPU port define CPU_SIZEOF_POINTER.  It must be an
>>> integer literal that can be used by the assembler.  This value will be
>>> used to calculate offsets of structure members.  These offsets will be
>>> used in assembler code.
>>>
>>> The size of a pointer is part of the application binary interface (ABI)
>>> and thus independent of the actual programming language.  The compiler
>>> will provide defines to determine the current ABI.  We use these 
>>> defines
>>> to select the appropriate CPU_SIZEOF_POINTER value.
>>>
>>> Static assertions in the new file "cpukit/score/src/percpuasm.c" will
>>> ensure that the value of CPU_SIZEOF_POINTER is consistent with the
>>> current compiler settings.  Also the offset values used by assembler
>>> code are verfied.
>>
>> Again, this approach lacks generality and is fundametally flawed.
>
> Please be more specific.  I don't understand what you mean.

The size of a pointers is not a per-cpu constant. It depends upon 
various compilation parameters/compiler arguments and can also vary 
between compiler versions and compilers.

=> Any hardcoded assumption on pointer sizes is wrong.

=> It cannot be encoded into _any_ general or per-cpu installed header. 
It can only be encoded into a per-BSP header.

>
>>
>> It's just a random accident it appears to work for some targets.
>
> Please provide a counter example.
>

In general, any target can exhibit changes to sizeof pointers.

Here are j3 real world examples:

# find -name config.h -exec grep -H '^#.*SIZEOF_VOID' {} \;
./h8300-rtems4.11/config.h:#define SIZEOF_VOIDP 2
./h8300-rtems4.11/h8300s/config.h:#define SIZEOF_VOIDP 4
./h8300-rtems4.11/h8300s/int32/config.h:#define SIZEOF_VOIDP 4
./h8300-rtems4.11/h8sx/config.h:#define SIZEOF_VOIDP 4
./h8300-rtems4.11/h8sx/int32/config.h:#define SIZEOF_VOIDP 4
./h8300-rtems4.11/h8300h/config.h:#define SIZEOF_VOIDP 4
./h8300-rtems4.11/h8300h/int32/config.h:#define SIZEOF_VOIDP 4
./m32c-rtems4.11/config.h:#define SIZEOF_VOIDP 2
./m32c-rtems4.11/m32cm/config.h:#define SIZEOF_VOIDP 4
./x86_64-redhat-linux/config.h:#define SIZEOF_VOIDP 8
./x86_64-redhat-linux/32/config.h:#define SIZEOF_VOIDP 4


> Independent of potential problems with this patch, I think it is 
> better than the current state.
>
I could not disagree more. All your patch does is to replace one broken 
and flawed approach which managed to furtherly infect RTEMS with 
another, similarly broken approach.

One way to avoid the whole problem would be to not use asm but to use 
c-inline-asm routines, to which you can pass c-constants.

Another, RTEMS specific way would be to move all such constants and 
routines outside of cpukit.


Ralf