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

Ralf Corsepius ralf.corsepius at rtems.org
Thu Nov 8 16:29:23 UTC 2012


On 11/08/2012 11:43 AM, Sebastian Huber wrote:
> On 11/08/2012 11:25 AM, Ralf Corsepius wrote:
>> 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.
>
> This is not a per CPU constant!  It is a per ABI constant.

You are nit picking at words. We are looking at the same topic from 
different angles.

>> => It cannot be encoded into _any_ general or per-cpu installed
>> header. It can
>> only be encoded into a per-BSP header.
>
> We already use the same mechanism e.g. on PowerPC to select the CPU
> context for SPE or AltiVec and also for soft-float vs. hard-float.

I don't know about SPE/AlitVec, but for soft/hard-float, this should not 
be the case.

These are supposed to be detected from preprocessor defines. If this 
doesn't apply anymore, RTEMS has regressed.

>>
>>>
>>>>
>>>> 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
>
> Yes, but have a look at the patch:
>
> [...]
> diff --git a/cpukit/score/cpu/h8300/rtems/score/cpu.h
> b/cpukit/score/cpu/h8300/rtems/score/cpu.h
> index ea9d443..1811ce7 100644
> --- a/cpukit/score/cpu/h8300/rtems/score/cpu.h
> +++ b/cpukit/score/cpu/h8300/rtems/score/cpu.h
> @@ -497,6 +497,12 @@ SCORE_EXTERN Context_Control_fp  _CPU_Null_fp_context;
>
>   #define CPU_STACK_MINIMUM_SIZE          (1536)
>
> +#if defined(__H8300H__) || defined(__H8300S__) || defined(__H8300SX__)
> +  #define CPU_SIZEOF_POINTER 4
> +#else
> +  #define CPU_SIZEOF_POINTER 2
> +#endif
> +
>   /*
>    *  CPU's worst alignment requirement for data types on a byte
> boundary.  This
>    *  alignment does not take into account the requirements for the stack.
> [...]
> diff --git a/cpukit/score/cpu/m32c/rtems/score/cpu.h
> b/cpukit/score/cpu/m32c/rtems/score/cpu.h
> index c1d1db8..d83e93d 100644
> --- a/cpukit/score/cpu/m32c/rtems/score/cpu.h
> +++ b/cpukit/score/cpu/m32c/rtems/score/cpu.h
> @@ -575,6 +575,12 @@ typedef struct {
>    */
>   #define CPU_STACK_MINIMUM_SIZE          (2048L)
>
> +#ifdef __m32cm_cpu__
> +  #define CPU_SIZEOF_POINTER 4
> +#else
> +  #define CPU_SIZEOF_POINTER 2
> +#endif

Hard-coding what could be automatically detected rsp. is compiler 
defined - A truely silly approach.

In longer terms this hard-coding is error-prone and unmaintainable. In 
fact you are duplicating and hard-coding what already is specified 
elsewhere.

>   /**
>    *  CPU's worst alignment requirement for data types on a byte
> boundary.  This
>    *  alignment does not take into account the requirements for the stack.
> [...]
>
>> ./x86_64-redhat-linux/config.h:#define SIZEOF_VOIDP 8
>> ./x86_64-redhat-linux/32/config.h:#define SIZEOF_VOIDP 4
>
> Yes, exactly a 32-bit ABI and a 64-bit ABI.
Again, you are nit-picking on words.

On the user side, this is all about sizes of types. ABIs are _completely 
irrelevant_.

>> 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.
>
> Who has the resources to do this?
In C you can use "sizeof(void)" and thus do not have to care about 
hard-code constants.

> Also I doubt that it is a good idea
> to replace the well tested existing assembler code with inline asm which
> is highly compiler specific and error prone.
Rubbish. It's trivial to verify such inline-asm code.

>> Another, RTEMS specific way would be to move all such constants and
>> routines
>> outside of cpukit.
>
> Why?  If you link a cpukit library with pointer size == 2 and a BSP
> library with pointer size == 4, then I don't think this works.
If properly written, such configuration will fail to build much earlier 
at least flood you with warnings. If not, compilation will succeed silently.

Ralf








More information about the devel mailing list