Assembly code help needed

[davarm]

I have written a very simple round robin tasker that changes tasks on a timer interrupt. The tasks control blocks (a struct) that holds nothing more than a stack, a stack pointer and a 'Next' pointer to the next task in the list. I also have a global pointer called 'CurrentTask' that points to the current task control block. In the interrupt I just want to preserve the stack pointer, move to the next task, get the new stack pointer and do an RTI. One would think this is simple enough to do but for some reason when I switch to the second task I end up somewhere I should not be. Can anyone see something I am missing?

typedef struct TCB
{
    char *StackPtr;
   struct TCB *Next;
   char  Stack[STACK_DEPTH];   // stack
};

struct TCB *CurrentTask;

ISR(TaskTimerInt_Interrupt)
{
    /* RTCSC: RTIF=1 */
    setReg8Bits(RTCSC, 0x80);            /* Reset real-time counter request flag */
 
    __asm pshh;                        // push H on the stack
    __asm tsx;                           // preserve the value of the stack pointer in H:X
    __asm sthx CurrentTask;    // store the stack pointer
   
    __asm ldhx CurrentTask;    // load H:X with the value pointed to by CurrentTask
    __asm aix #2;                      // add 2 to equal the location of Next (see typedef)
    __asm sthx CurrentTask;   // store the Next value into CurrentTask
   
    __asm ldhx CurrentTask;   // load the CurrentTask stack pointer value in H:X
    __asm txs;                          // store H:X into the SP
    __asm pulh;                        // pull H
    __asm rti;                           // return from interrupt
}

[tonyp]

Your example looks like you're only trying to switch between two tasks using a single variable for keeping the other task's stack pointer.  I attach an old example for doing exactly this.

If you want to have more than two tasks, you need to setup an array with as many pointers as the number of tasks.

You also need a variable to tell you which task is your currently executing one, for example, TaskHandle

Then, increment this index (wrapping around as needed) and use it as an index to the array of SPs.  Something like this (completely untested example) might work:

SWITCHER:
 pshh

 tsx
 pshhx

 lda TaskHandle
 ldhx #StackArray
 aax ;(macro for adding A to HX)
 pula
 sta ,x
 pula
 sta 1,x

 lda TaskHandle
 inca
 cmpa #MAXTASKS
 blo ?
 clra
? sta TaskHandle

 ldhx #StackArray
 aax ;(macro for adding A to HX)

 ldhx ,x
 txs

 pulh
 rti

Good luck.

[davarm]

Thanks for all the great quick help.

I think I need to explain more.

I use the linked list approach so I can add and remove tasks as needed.

The instruction 'sthx CurrentTask' should (in my mind) store the SP in the Current->StackPtr location because it is the first value in the struct so it resides at the address of CurrentTask.

Maybe the better question to ask is:

How do I write the equivalent  of 'sthx Current->StackPtr' and 'ldhx Current->StackPtr' in assembly language?

[tonyp]

Very similar to what I gave above.

 ldhx Current
 ldhx StackPtr,x
 ...
 txs

to load Current -> StackPtr

and

 tsx
 pshhx
 ..
 ldhx Current
 pula
 sta StackPtr,x
 pula
 sta StackPtr+1,x
 
to do the saving.

---

davarm wrote:

How do I write the equivalent  of 'sthx Current->StackPtr' and 'ldhx Current->StackPtr' in assembly language?

---

[davarm]

I think I understand this now. You are using the IX2 addressing mode which loads an address (in this case CurrentTask) into H:X and then uses StackPtr as an offset into CurrentTask.

I think the lights are coming on now.

[davarm]

Ok, I am feeling really stupid. I have written thousands of lines of assembler for three different (32) processors and I have never had this much trouble doing something so (what I think) simple.

Thanks to Mr. TonyP I understand a little more of this (for me) new world of 8-bit microcontrollers. It is definitely a different way of thinking.

Based on what Tony mentioned above I have written the following code in my C code TimerISR:

extern struct TCB *CurrentTask;
...

ISR(TaskTimerInt_Interrupt)
{

    __asm pshh;   
    __asm tsx;
    __asm pshx;                                  // for some reason my compiler does not like pshhx
    __asm pshh;
    __asm ldhx CurrentTask;
    __asm pula;
    __asm sta StackPtr, x;                  // I get "undefined class/struct/union" error when I compile
    __asm pula;
    __asm sta StackPtr + 1, x;

Looks to me like there is some trick to declaring CurrentTask->StackPtr or refering to the StackPtr element struct TCB. I put it as the first element in the struct so I could just refer to CurrentTask as an address.

Does StackPtr need to be declared in some tricky way?

Also, a related question. When I am writting my embedded __asm code if I stop and write a line of C code such as:

CurrentTask = CurrentTask->Next;

I get the same "undefined struct/class/union" error. I am sure they are related.

Is "extern" not enough?

thanks for everyone's patience,

dave

[bigmac]

Hello Dave,

There seems to be a few issues within your code.  Firstly, I assume your ISR() function is not really an interrupt service routine, but possibly a function that might be called by one.  The usual format for an ISR would be

interrupt void ISR_func( void)

{

}

Any parameters required within the ISR would need to be associated with global or static variables.

Be aware that HLI assembler is somewhat different than normal assembler.  Instructions with a calculated offset have a different format.  However, I don't think it is really necessary to use the indexed instructions, that seem to be problematic (see below).

I would assume that you are going to need a block of RAM memory for each of the tasks - perhaps an array of structures should be defined for this purpose.  The CurrentTask pointer variable does not presently seem to be initialised to point to a particular block of memory.

The following modified code seems to produce the correct outcome for storing the stack pointer, under simulation.  The structure is slightly modified to utilize the typedef for the global variable definitions.

typedef struct tcb
{
   char *StackPtr;
   struct tcb *Next;
   char  Stack[STACK_DEPTH];   // stack
} TCB;

 
// Global variables:

TCB *CurrentTask;              // Pointer to structure
TCB TaskTab[MAX_TASKS];        // Array of structures

void set_task( void)
{
   char *StackPtr;
  
   __asm tsx;
   __asm sthx  StackPtr;
  
   CurrentTask->StackPtr = StackPtr;

   ...

}

set_task() function usage:
   
   CurrentTask = &TaskTab[task_ID];  // Initialise pointer
   set_task();

Regards,

Mac

[davarm]

I got it working!

Thanks to CompilerGuru, bigmac and especially tonyp.

First off my philosophy was that I wanted to create a scheduler that was extensible in the sense that adding more tasks was as easy as calling the CreateTask api. I did not have alot of code overhead that was required if I use an array. Each task occupies 8 bytes plus the size of the stack you choose. (my final implementation was a little more expanded than from what I had above) The benefit is that I can add and delete tasks on the fly which is something I am going to need. I am sure I have reinvented the wheel somewhere but I was not able to find something that worked for my needs. The tasker called Helium came close. I have attached all the "important parts" of my final implementation.

Code:

// ****************************************************************************#define MAX_TASKS                    8#define STACK_DEPTH                 30#define SIZEOF_SWITCH_STACK_DROP     6// ****************************************************************************typedef struct TCB{    char *StackPtr;    struct TCB *Next;    struct TCB *Prev;    char  Stack[STACK_DEPTH];   // stack    int  Task;               // address of task function};struct TCB *Head;struct TCB *Tail;struct TCB *CurrentTask;// ****************************************************************************void TaskCreate(void (*Task)()) {    struct TCB *NewTCB = NULL;    NewTCB = malloc(sizeof(struct TCB));    if (NewTCB == NULL)    { return;    }     // save the task address so TaskDestroy can find it     NewTCB->Task = (int)Task;    // put the task on the stack        *(int*)((NewTCB->Stack) + (STACK_DEPTH - 2)) = (int)Task;        // load the stack pointer accounting for the rti in the task switch        NewTCB->StackPtr = (NewTCB->Stack) + (STACK_DEPTH - SIZEOF_SWITCH_STACK_DROP);        // if this is the first element in the list        if (Head == NULL)    { // make it point to itself      Head = NewTCB; Tail = NewTCB;   // this is the first task to run // so load CurrentTask with this TCB   CurrentTask = NewTCB;     }    // make all the linked list pointers point to the right things        NewTCB->Prev = Tail;    Tail->Next = NewTCB;    NewTCB->Next = Head;    Tail = NewTCB;}// ****************************************************************************void TaskDestroy(void (*Task)()){    struct TCB *TCBItem = Head;    do    { if ((int)Task == TCBItem->Task) {     TCBItem->Prev->Next = TCBItem->Next;     TCBItem->Next->Prev = TCBItem->Prev;     free(TCBItem);     return; }    }  while (TCBItem->Next != Head);}// ****************************************************************************ISR(TaskTimerInt_Interrupt){    /* RTCSC: RTIF=1 */    setReg8Bits(RTCSC, 0x80);            /* Reset real-time counter request flag */          // No need to pshh since this C function has already done it    // Save the current stack pointer in CurrentTask->StackPtr            __asm tsx;                  // save the SP in H:X    __asm pshx;                 // push SP low byte    __asm pshh;                 // push SP high byte    __asm ldhx CurrentTask;     // load the address of CurrentTask    __asm pula;                 // pull the top value from the stack in A    __asm sta , x;              // store A (really H) where H:X is pointing (CurrentTask->StackPtr)    __asm pula;                 // pull the top value from the stack    __asm sta 1, x;             // store A (really X) where H:X + 1 is pointing (CurrentTask->StackPtr + 1)        // Now point to the next node in the task linked list            __asm ldhx CurrentTask;    // load H:X with the address of CurrentTask    __asm aix #2;              // add 2 to equal the location of Current->Next    __asm ldhx , x             // load H:X with what is pointed to by CurrentTask    __asm sthx CurrentTask;    // store the address of the next task in CurrentTask        // H:X already has the address of CurrentTask in them    // Load the next task's stack pointer        __asm ldhx ,x              // load what is pointed to by H:X which is Current->StackPtr    __asm txs;                 // store H:X into the SP    __asm pulh;                // pull H    __asm rti;                 // return from interrupt}// ****************************************************************************void main(void){    /*** Processor Expert internal initialization. DON'T REMOVE THIS CODE!!! ***/    PE_low_level_init();    /*** End of Processor Expert internal initialization.                    ***/    Head = NULL;    TaskCreate(Task1);    TaskCreate(Task2);    // simulate a task switch    // this is the exact code as at the end of the ISR        __asm ldhx CurrentTask;    // load H:X with the address of CurrentTask    __asm ldhx , x             // load H:X what is pointed to by CurrentTask which is the StackPtr    __asm txs;                 // transfer H:X to stack pointer register    __asm pulh                 // pulh to emulate the task switch code    __asm rti;                 // return from interrupt, i.e. execute the task}// ****************************************************************************

 

Thanks again for the education,

dave

[tonyp]

A couple of comments.

1. Although it really depends on your code, for any serious application a STACK_DEPTH of 30 is probably suicidal.

2. When pointing to Next, you can optimize the code you have a bit.  HX is already loaded with CurrentTask  And AIX #2 can be part of the following indexed instruction, like so:

    // Now point to the next node in the task linked list
       
>>> __asm ldhx 2,x             // load H:X with what is pointed to by CurrentTask
    __asm sthx CurrentTask;    // store the address of the next task in CurrentTask

[davarm]

Good advice.

I pulled the 30 from a certain "dark" place just for testing.

I am wide open to an experienced suggestion as to a suitable stack depth.

My application is just to communicate with peripherals when they need it.

I only have 4K RAM total on my chip.

Please suggest.

dave

[tonyp]

I use assembly and have better control of the compiled environment.  I don't know how easy this would be with a C compiler.  Here's what I do:

After all RAM sections in all modules have been included (and processed by the assembler), I calculate the remaining available RAM.  Then divide this by the MAX_TASKS and have my per task stack size.  Using my assembler:

 #RAM

?RAM_USED

TASK_STACK_SIZE equ RAM_END-?RAM_USED/MAX_TASKS

Voila.  (Note: With this assembler there is an implied parenthesis around the subtraction.)

Because I tend to use very little global variable space, this usually leaves me with a few (about 5) hundred bytes per task (on a QE128 with 8K RAM) with about ten defined tasks.  In practice, I've found anything below 100 bytes per task to be problematic but it all depends on how deep your code gets (functions calling other functions, and the size of local parameters and variables these functions use.)


Message Edited by tonyp on 2008-10-30 12:58 AM

[tonyp]

The extern only defines CurrentTask as being defined elsewhere.  But don't you still need to define the TCB struct (of which StackPtr is part) to be allowed to use it?

Anyway, StackPtr is just an offset within the record/structure.  If you can't figure out how to use the compiler to get this offset symbolically, just use the numeric offset you already know.  If StackPtr is first in the record (struct), use zero for offset:

  ldhx ,x

and

 sta ,x
 sta 1,x

Of course, it'd be nicer if you could use the identifier directly someway, but I don't know how you would do this with your compiler, as I don't use it.  Someone using the same compiler as you (CodeWarrior?) may be able to help you with this.  You would need to somehow tell the compiler to return the offset StackPtr represents.  The manual should have a section dedicated to linking inline assembly code with the parent C program.

By the way, I use PSHHX and PULHX so often in my programs I forgot to mention that PSHHX is a macro (ie., not an official instruction) and your compiler rightfully complains.

[CompilerGuru]

The store and restore of the stack pointer just stores the SP in the global CurrentTask and not in
the StackPtr field CurrentTask points to.
Right now I don't have the time to code it.

Daniel