Detecting compile-time versus run-time incompatibilities in powerPC

[Till Straumann]

First of all, IMO the only register definitions, access macros etc.
that should be in cpukit headers are such that are reflecting the
multilib variants (i.e., describe features that are 'known' to gcc
also).


E.g., (IMHO), there can be

#ifdef __ALTIVEC__
#define xxx
#endif

because the -maltivec switch lets the compiler
  * emit altivec instructions
  * defined __ALTIVEC__
  * possibly change the ABI

=> A multilib variant is justified and can be detected via CPP symbol.

In your case, creating a new multilib variant (remember that everything
in cpukit will eventually be multilibbed) just because you want
to compile your code below for a 5553/5554 is unjustified.

Hence, there is no place for #if MPC55XX_CHIP_TYPE or a declaration
of CBDR or similar registers in cpukit (I consider any existing macros
of this type obsolete - they are there for historical reasons).

Now, if you want to introduce such macros in 'libcpu' and/or 'libbsp' then
I have less strong objections. I still think as much code as possible should
use run-time detection or at least a cpu specific library but you could
use conditionals but in NO case should the symbol that defines the CPU
you want to compile for be defined by gcc/cpp/multilibs. The correct
way would be using 'configure' for that purpose.

You would 'configure' libcpu for a specific target cpu and let configure
produce appropriate variants of headers etc.

-- Till

PS: I have two comments regarding your code example below:

1) It would obviously be easy to handle the examples below at run-time
    -- provided that there is a 'cpuid' register of some sort, of course.

2) IMHO it is bad programming practice to access registers just
    as ordinary memory. I would always use dedicated I/O operations
    for sake of readability and to ensure/enforce ordering.

Peter Dufault wrote:
> On Apr 12, 2010, at 10:52 , Till Straumann wrote:
>
>   
>> I don't think it is a good idea.
>>
>> If you introduce such tests (and in the past there have been many
>> which we have tried to eliminate) then it will become much
>> harder to ensure that the source even compiles cleanly for all
>> sensible combinations of CPP symbols.
>>
>> Plus, some people might be tempted to introduce multilib variants
>> for all such combinations which is IMHO a wrong approach which
>> leads to rapid growth.
>>     
>
> Let me present the cases at hand, and you can suggest a good way to address them.  One is a "Set unused registers" that writes to a register that causes an exception on the MPC5553/MPC5554:
>
>   /* Set unused registers */
>   regs->CBDR.R = 0;
>   regs->CCNTR.R = 0;
> #if MPC55XX_CHIP_TYPE != 5554 && MPC55XX_CHIP_TYPE != 5553
>   /* This is reserved on the MPC5553/MPC5554.
>    */
>   regs->ALTCADR.R = 0;
> #endif
>
> In this case maybe "if it's unused leave it alone" works and the code can be removed, but for now I need to do something that will not break any other platforms, so I have to put in that conditional.
>
> The second example is changing a clock mode to a buffered mode.  Apparently it's the only mode available on some chips, but it is unsupported on the MPC5554 (though it is supported on the MPC5553).  And there are no exceptions or anything on the MPC5554, interrupts just stop working.
>
> #if MPC55XX_CHIP_TYPE == 5554
>   ccr.B.MODE = MPC55XX_EMIOS_MODE_MC_UP_INT_CLK;
> #else
>   ccr.B.MODE = MPC55XX_EMIOS_MODE_MCB_UP_INT_CLK;
> #endif
>
> How and where do you summarize these differences, so people know what to look out for?  How do you quickly catch incompatible changes to the code base?  I think the headers are a reasonable place.  I'd like to have a conditional to undefine unsupported registers and modes so that code that adds e.g. the buffered timer mode back in will fail to compile for me and I'll find it right away. If it's going to be MPC55XX only for now I'd have something like "MPC55XX_UNDEFINE_UNSUPPORTED" so that the header would have:
>
> /* Buffered modes are not supported on the '5554 */
> #if !(defined(MPC55XX_UNDEFINE_UNSUPPORTED) && MPC55XX_CHIP_TYPE == 5554)
> #define MPC55XX_EMIOS_MODE_MCB_UP_INT_CLK 80U
> #define MPC55XX_EMIOS_MODE_MCB_UP_EXT_CLK 81U
> #endif
>
> Peter
> -----------------
> Peter Dufault
> HD Associates, Inc.      Software and System Engineering
>
>