HC08: !@#$?%& Switch

[admin]

Has anyone noticed that CW seems to compile a C "switch" statement very poorly?  It looks like it converts the arguement into an "int" then do some sort of division into a table (presumeably to account for the fact that you might have 65,535 "case" statements).  Has anyone been able to have an efficient switch, or should I just use a bunch of if...elseif...else statements?

BTW, I'm using CW for the HC08.

Thanks,

Tomahawk

Message Edited by CrasyCat on 2007-04-13 02:19 PM

[BlackNight]

Hello,

to answer the question why things are done as 'int': ANSI C specifies that the for 'switch(expression)' the expression has to be int. The compiler is free to optimize this as long as the behaviour is still the same as it would be an int expression.

And that could be tricky.

In general I think the compiler does a good. But the compiler is using some heuristics for a good choice, and as for any choices, the choice might be not be optimal.

BK

[bigmac]

Hello,

With the use of the enum type, the switch variable is assumed to be 16-bits.  If I change to an 8-bit switch variable, it seems that this is not promoted to 16 bits,  thus giving slightly more efficient code.  But in this case, the savings amount to only a few bytes.

For the 16-bit case, the assembly instructions ldhx <variable> and cphx #<case> are used.  For the 8-bit case, lda <variable> and cmp #<case> are used instead.  I should mention that I am using CW 6.3.

Regards,

Mac

[Lundin]

For the code in Andrei's example, the compiler should be able to optimize it to an 8-bit version, since it can statically determine that the enum can only have values 0-5. While the enum variable in itself, "nDisplayState", must be equivalent to an int by the standard, the enum constants can be equivalent to 8-bit,  since the enum constants' size is implementation-defined behavior in C.If the enum constants are 8-bit, then I think the compiler should be able to optimize the whole switch into 8-bit.

You could of course force CW to make the enum type 8-bit as well, by adjusting the type sizes. But then the code wouldn't be standard compliant any longer.

[bespenschied]

I also stopped using switch. The first time I tried it my code exploded to about 5 times what it was and wasted a LOT of space.

[AndreiLuchuk]

bringing this back from the nether... WHY can't I use switch() in CodeWarrior?

targeting : mc9s08pt32, CW10.2 with MCU Update 1.0.0 using FullChip Simulator.

stepping in debug, it just skips the whole switch(){} routine and takes me to void _Jump_Table_Header_Offset(void) in rtshc08.c. what gives?

here is the code:

#include "StdDefs.h"#include "FormatDisplay.h"typedef enum {   Display_State_Idle,   Display_State_Init,   Display_State_Normal,   Display_State_End,   Display_State_Err,   Display_State_Delay,} EDisplayStates;EDisplayStates nDisplayState = Display_State_Idle;void FormatDisplay(void){   switch(nDisplayState)   {      case Display_State_Idle:         break;      case Display_State_Err:         //how did we get to this state??         break;      default:         break;   }   return;//RTS }

[AndreiLuchuk]

ok, update, I simplified the code and added a LED toggle from TWR demo... 

#include "mc9s08pt60.h"#include "TWR-60.h"// Steps in Display sequencetypedef enum {   Display_State_Idle = 0,   Display_State_Init = 1,   Display_State_Normal = 2,   Display_State_LRV = 3,   Display_State_HRV = 4,   Display_State_EU = 5,   Display_State_Clo = 6,   Dispaly_State_Chi = 7,   Display_State_Lcd = 8,   Display_State_End = 9,   Display_State_Err = 10,   Display_State_Delay = 11,   Display_State_UpdateLCD = 12} EDisplayStates;// Steps in Substate sequencetypedef enum{   Substate_Idle = 0,   Substate_Main = 1,   Substate_CurrentValue = 2,   Substate_Leading_1 = 3,   Substate_First_Digit = 4,   Substate_Second_Digit = 5,   Substate_Third_Digit = 6,   Substate_DP = 7,   Substate_Trailing_Zero = 8,   Substate_K = 9,   Substate_Minus = 10,   Substate_Units = 11,   Substate_AllOn = 12,   Substate_AllOff = 13,   Substate_Cleanup = 14,   Substate_Yes = 15,     Substate_Err = -1} ESubstates;// Steps in LCD Init sequencetypedef enum{   Init_Idle = 0,   Init_AllOn,   Init_Delay,   Init_AllOff} EInitStates;// User Switch events, after debounce and buffering typedef enum {   Left,   Right,   none} ESwitchEvents;EDisplayStates mDisplayState = 0;                  //possible statesESubstates mSubstate = 0;                          //possible substatesEInitStates mInitState = Init_Idle;                //LCD init statesESwitchEvents mSwitchEvent = none;                 //user switch events buffervoid InitLCD(void);                 // toggle all LCD segments on for 6 seconds, then off.void ProcessRangeValue(UINT8);void ProcessEngineeringUnits(void);void ProcessCalibrateLow(void);void ProcessCalibrateHigh(void);void DisplayBuffer(void);void FormatDisplay(void){   switch(mDisplayState)   {      case Display_State_Idle:         LED0_Toggle();         mDisplayState++;         break;      case Display_State_Init:         LED0_Toggle();         InitLCD();         break;      case Display_State_Normal:         LED0_Toggle();         switch(mSwitchEvent)         {            case Left:               mDisplayState++;               break;            case Right:               break;            case none:break;         }         mSwitchEvent=0;         break;      case Display_State_LRV:         LED0_Toggle();         break;      case Display_State_End:         //display "End" on LCD         switch(mSwitchEvent)         {            case Right:               mDisplayState = Display_State_Normal;               break;            case Left:               mDisplayState = Display_State_LRV;               break;            default:break;         }         mSwitchEvent=0;         break;      case Display_State_Err:         break;      default:         mDisplayState = Display_State_Normal;         break;   }   //RTS   return; }void InitLCD(void){   LED0_Toggle();   switch(mInitState)   {      case Init_Idle:         LED0_Toggle();         mInitState++;         break;      case Init_AllOn:         LED0_Toggle();         mInitState++;         break;      case Init_Delay:         LED0_Toggle();         break;      case Init_AllOff:         LED0_Toggle();         mInitState=0;         mDisplayState = Display_State_Normal;         break;   }   return;}

whats funny now, is that even though my Enums are init'ed, in debug they do not come up at state 0, mSubstate is at 3, and mInitState is -1! And I still can't get the LED to toggle at each step!

[CrasyCat]

Hello

  Did you link your application with the standard ANSI C startup or with the minimal startup?

  In other word did you specify the option -D__ONLY_INIT_SP in your compiler command line?

  If this is the case, the global variables defined with an initializer will not be set to their initialization value.

  You have 2 ways to fix that:

       1- Remove option -D__ONLY_INIT_SP from your compiler option and rebuild the application (including the startup code)

        2- Make sure to initialize all global variables using assignment at the beginning of function main.

CrasyCat

[Lundin]

Or better: never ever use global variables in any C program. Instead use the OO concept of private encapsulation. And at the same time, ensure that your code gets portable to all embedded C compilers that almost all of them have the very common non-standard extension "skip static initialization."

static EDisplayStates mDisplayState ;                  //possible statesstatic ESubstates mSubstate ;                          //possible substatesstatic EInitStates mInitState ;                //LCD init statesstatic ESwitchEvents mSwitchEvent ;                 //user switch events buffer

...

void InitLCD(void){
  mDisplayState = 0;
  mSubstate = 0;
  mInitState = Init_Idle;
  mSwitchEvent = none;
...
}

[AndreiLuchuk]

why not as part of module code, instad of init?

static EDisplayStates mDisplayState = 0;static ESubstates mSubstate = 0;static EDigits mDigit = D_none;

 also, I cannot find  -D__ONLY_INIT_SP in the compiler reference, and I'm doing ANSII init (although, doing a Diff on minimal and ANSII, I cannot find any differences??)

[Lundin]

The C standard guarantees that all objects with static storage duration are initialized before the program starts. Variables with static storage duration are: everything you declared as static, as well as all global variables.

Such static storage duration variables will either be initialized to the value you explicitly picked for them, such as EDigits mDigit = D_none;. If you didn't explicitly write an init value, they will be initialized to zero.

On a non-volatile memory system such as a flash MCU, the static initialization is done in runtime, after MCU reset, before main() is called. This is done in a loop where the values are copied down from flash to RAM. In Codewarrior, you will find some mysterious code in start08.c doing this, it is typically referred to as "copy-down" or similar.

There is a compiler switch enabling this copy-down functionality if you picked "ANSI" startup when you created the project.

("ANSI C" in Codewarrior means the C standard ISO 9899:1990. The Codewarrior devs are a bit confused about standard nomenclature and still speak of "ANSI C", even though the C language has been an ISO standard for 22 years.)

However, there are many cases in embedded systems where you don't want this tedious copy-down to be executed at startup, taking up time. You might have more important tasks that you want to do immediately after startup, such as setting up watchdogs, low-voltage detects, I/O registers etc etc. Therefore, almost every embedded compiler has a non-standard option to abandon the C standard's requirement of static initialization. In Codewarrior this is called "minimal startup". If you enabled this option, no initialization of statics/globals will take place. Instead you have to set them explicitly in runtime.

This is why I made the proposed change, to make your code portable/compatible with all common C compilers for embedded systems. It is a good idea never to rely on global/static initialization when writing code for embedded systems.

[AndreiLuchuk]

Allright, I'm making a new bareboard project, calling it EUM_60 (because it will run on the TWR-60 dev board) with the mc9s08pt60, using MultiLink, C, no Rapid Applicaiton Development, Tiny memory model, no float support, and minimal startup.

main.c

#include <hidef.h> /* for EnableInterrupts macro */#include "derivative.h" /* include peripheral declarations */#include "CPU.h" //MCU_Init()#include "TWR-60.h" //LED and Button Init#include "FormatDisplay.h" //FormatDisplay state logicvoid main(void) {   DisableInterrupts;   MCU_Init();              //MCU initialization   EnableInterrupts;      LED_Init();   Button_Init();      mDisplayState = Display_State_Idle;   mInitState = Display_State_Idle;      for(;;)    {      FormatDisplay();   } }

 FormatDisplay.c

#include "FormatDisplay.h"#include "mc9s08pt60.h"#include "TWR-60.h"//L O C A L    F U N C T I O N    P R O T O T Y P E S      void InitLCD(void);// S T A R T    O F    L O G I C =======================void FormatDisplay(void){   switch(mDisplayState)   {      case Display_State_Idle:         LED0_Toggle();         mDisplayState = Display_State_Init;         break;      case Display_State_Init:         LED1_Toggle();         InitLCD();         break;      case Display_State_Normal:         break;      case Display_State_End:         mDisplayState++;         break;      case Display_State_Err:         mDisplayState = Display_State_Init;         break;      default:         LED3_Toggle();         mDisplayState = Display_State_Idle;         break;   }   return; }void InitLCD(void){   LED0_Toggle();   switch(mInitState)   {      case Init_Idle:         LED2_Toggle();         mInitState++;         break;      case Init_AllOn:         LED2_Toggle();         mInitState++;         break;      case Init_Delay:         LED2_Toggle();         mInitState++;         break;      case Init_AllOff:         LED2_Toggle();         mInitState++;         break;   }   return;}

 FormatDisplay.h

#ifndef FORMAT_DISPLAY_H#define FORMAT_DISPLAY_H                        //E N U M E R A T I O N Stypedef enum // Steps in Display sequence{   Display_State_Idle,   Display_State_Init,   Display_State_Normal,   Display_State_End,   Display_State_Err} EDisplayStates;typedef enum // Steps in LCD Init sequence{   Init_Idle,   Init_AllOn,   Init_Delay,   Init_AllOff} EInitStates;//M O D U L E   V A R I A B L E S      static EDisplayStates mDisplayState;static EInitStates mInitState;    //F U N C T I O N    P R O T O T Y P E S      void FormatDisplay(void);#endif

 As you can see I have static enums, with variable initialization in main. However, I'm still getting the same problem. mDisplayState and mInitState are not initialized in FormatDisplay! without a default case in Init, it doesnt even process the switch! what am I missing here? should I use tables?

[AndreiLuchuk]

I resolved it by putting in the main

mDisplayState = Display_State_Idle;
mInitState = Display_State_Idle;

in the module

EDisplayStates mDisplayState;EInitStates mInitState;

in the module header.

extern EDisplayStates mDisplayState;extern EInitStates mInitState;

but i still dont understand why. 

[Lundin]

Static variables and functions are very fundamental mechanisms of C programming. Here's tutorial of C programming for microcontrollers: http://users.ece.utexas.edu/~valvano/embed/toc1.htm. It even uses a Freescale MCU for reference, even though the same mechanisms apply to all MCUs.

[bigmac]

Hello Lundin,

There seems to be a problem with your previous link.

Regards,

Mac

[J2MEJediMaster]

The link happened to pick up the period on the end. It was a slip-up on the selection in the editor, I suspect. Removing the period makes the link work:

http://users.ece.utexas.edu/~valvano/embed/toc1.htm

---Tom

[AndreiLuchuk]

thank you gentlemen, this is exactly the type of information i need. i will also be looking through the information resources listed in CodeWarrior reference manual: 

• American National Standard for Programming Languages – C, ANSI/ISO 9899–1990 (see ANSI X3.159-1989, X3J11)
• The C Programming Language, second edition, Prentice-Hall 1988
• C: A Reference Manual, second edition, Prentice-Hall 1987, Harbison and Steele
• C Traps and Pitfalls, Andrew Koenig, AT&T Bell Laboratories, Addison-Wesley Publishing Company, Nov. 1988, ISBN 0-201-17928-8
• Data Structures and C Programs, Van Wyk, Addison-Wesley 1988
• How to Write Portable Programs in C, Horton, Prentice-Hall 1989
• The UNIX Programming Environment, Kernighan and Pike, Prentice-Hall 1984
• The C Puzzle Book, Feuer, Prentice-Hall 1982
• C Programming Guidelines, Thomas Plum, Plum Hall Inc., Second Edition for Standard C, 1989, ISBN 0-911537-07-4
• DWARF Debugging Information Format, UNIX International, Programming Languages SIG, Revision 1.1.0
• DWARF Debugging Information Format, UNIX International, Programming Languages SIG, Revision 2.0.0
• System V Application Binary Interface, UNIX System V, 1992, 1991 UNIX Systems Laboratories, ISBN 0-13-880410-9
• Programming Microcontroller in C, Ted Van Sickle, ISBN 1878707140
• C Programming for Embedded Systems, Kirk Zurell, ISBN 1929629044
• Programming Embedded Systems in C and C ++, Michael Barr, ISBN 1565923545
• Embedded C, Michael J. Pont, ISBN 020179523X

[bigmac]

Hello,

A static variable does not have global visibility, but its scope is restricted only to the file in which the variable is defined.  This restriction is the primary reason for using a static variable.  Therefore, the variable needs to be both defined and initialised from within the same file.  The variable that was initialised within main() was assumed by the compiler to be a totally separate (global) variable of the same name.

The use of extern is not appropriate for a static variable.

Regards,

Mac

[bigmac]

Hello,

The modified method that Lundin proposed did not require any ANSI initialisation, but explicitly initialised the variables within the function.  Your method assumes ANSI initialisation to be operative, when you provide an initial value at the point where the enum variable is defined.

Surely it would be more meaningful if you use the name of the initial enumeration state.  Otherwise there is little point in using an enumeration variable if you use a numeric value in lieu of the name.

mDisplayState = Display_State_Idle;  // Explicit initialisation

Additionally, when you define the enumeration states within the typedef's, with one exception the default allocation of enumeration values should correspond exactly with your explicit allocation.  So no need for the explicit allocations.  The single exception to this would be

Substate_Err = -1

---

Andrei Luchuk wrote:

 also, I cannot find  -D__ONLY_INIT_SP in the compiler reference, and I'm doing ANSII init (although, doing a Diff on minimal and ANSII, I cannot find any differences??)

---

The differences will occur within the start08.c file.  This contains the ANSI initialisation functions.

Regards,

Mac

[BlackNight]

Hello,

looking at your code, as a compiler I would detect that this switch does nothing. So I would get rid of that code.

I compiled your code, and indeed the compiler is doing the same: FormatDisplay() gets reduced to a single RTS instruction.

So it looks like this is not what you have. As you say it goes into _Jump_Table_Header_Offset().

This is actually a runtime routine called by the compiler to process the switch: with a jump table having a header with offsets (where to jump) in it.

Maybe have a look at the assembly code, but it looks to me that what you are debugging on the source side is not what you get in the target (well: simulator). Could it be that you have not compiled your latest and greates sources (or you have not set 'build before debug')? Just thinking what your problem could be.

Hope this helps,

BK

[Lundin]

CW doesn't produce very effective code for HC08, but I'd say that is mainly because of the CPU's limited architecture.

switch-statements in general are not effective. If the integers used in the switch-statement are adjacent, you could optimize them yourself by using a table of constant function pointers. Instead of

switch(x)
{
case SOMETHING:
func();
...
}

you could do

const FuncPtrType switchTable[max] = {func, ... };
...

switchTable[x]();