"Butter bei de Fische" was: Re: [PATCH v3] score: PR1607: Add and use CPU_SIZEOF_POINTER

Fri Nov 9 07:44:23 UTC 2012

Ralf, Sebastian,

we had that discussion for a long time now, with different flavors. From 
my knowledge of the PPC assembly code snippets, I can't see how we can 
come along without preprocessor symbols specifying the pointer length 
(Well, we might hard code it to 4 bytes, but that is not what we want...)

Part of the assembly code is highly optimized with respect to access 
order due to caching optimization etc. Dropping this would result in 
longer task switching times, which would directly reduce the system 
performance.

Ralf, you seem to have a concept in mind how that code could be written 
in a way, that is based on inline asm support instead of preprocessor 
macros. Maybe the whole discussion would become more transparent, if a 
certain piece of code is transformed the way you seem to vision.

Sebastian, Ralf, can you both identify a typical piece of assembly code, 
that might be recoded into a more portable code? We could then consider, 
which versions
- meet the portability requirements
- meet the performance requirements
- are readable and maintainable
- interface properly to the C portion of the code

Otherwise I fear that two opinions stay existent and will never resolve 
into a common result.

wkr,

Thomas.

Am 08.11.2012 17:29, schrieb Ralf Corsepius:
> 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
>
>
>
>
>
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel

-- 
--------------------------------------------
Embedded Brains GmbH
Thomas Doerfler           Obere Lagerstr. 30
D-82178 Puchheim          Germany
email: Thomas.Doerfler at embedded-brains.de
Phone: +49-89-18908079-2
Fax:   +49-89-18908079-9