Too many switch cases crashes MIOS?

[sneakthief]

Because I couldn't find a better way to do this (newbie alert!), I'm using 50 switches to load one of 50 different arrays.

However SDCC is giving gives these warnings when I compile:

-------------------------------------------------------------------

MOVFF PREINC1, r0x31

pcoderegs.c:367: removing reg r0x31 because it is only used once

MOVFF PREINC1, r0x32

pcoderegs.c:367: removing reg r0x32 because it is only used once

-------------------------------------------------------------------

Now here comes the bad part: when I trigger case 25, it crashes - even when there's nothing in the case! And switch 27 and 28 simply corrupt my array. All others work.

If I use less only 25 switches, I only get this SDCC warning:

-------------------------------------------------------------------

MOVFF PREINC1, r0x00

pcoderegs.c:367: removing reg r0x00 because it is only used once

-------------------------------------------------------------------

Aynhow, here's my crappy code that creates the error:

////////////////////////////////////////////////////////////////////////////////////////////// 
//  This function loads a stored sequence into the current song
//
//    _songload is the song number passed to this function
//
//    mclock_ctr_loadseq is used as a buffer that holds the current song info   
//
//    sq01 - sq50 are constant arrays with 20 elements - they hold note sequences
//  
////////////////////////////////////////////////////////////////////////////////////////////

void MCLOCK_LoadSeq(unsigned char _songload)
{

unsigned int i;

  switch(_songload) {

   case 0:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq01[i];  
    }
   break;

   case 1:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq02[i];  
    }
   break;

   case 2:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq03[i];  
    }
   break;

   case 3:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq04[i];  
    }
   break;


   case 4:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq05[i];  
    }
   break;

   case 5:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq06[i];  
    }
   break;


   case 6:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq07[i];  
    }
   break;

   case 7:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq08[i];  
    }
   break;

   case 8:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq09[i];  
    }
   break;

   case 9:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq10[i];  
    }
   break;

   case 10:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq11[i];  
    }
   break;

   case 11:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq12[i];  
    }
   break;

   case 12:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq13[i];  
    }
   break;

   case 13:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq14[i];  
    }
   break;

   case 14:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq15[i];  
    }
   break;

   case 15:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq16[i];  
    }
   break;

   case 16:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq17[i];  
    }
   break;

   case 17:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq18[i];  
    }
   break;

   case 18:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq19[i];  
    }
   break;

   case 19:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq20[i];  
    }
   break;

   case 20:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq21[i];  
    }
   break;

   case 21:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq22[i];  
    }
   break;

   case 22:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq23[i];  
    }
   break;

   case 23:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq24[i];  
    }
   break;

   case 24:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq25[i];  
    }
   break;

   case 25:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq26[i];  
    }
  break;

   case 26:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq49[i];  
    }
   break;

   case 27:
     for (i = 0; i < 20; i++) {
    mclock_ctr_loadseq[i] = sq50[i];  
    }
   break;

   case 28:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq29[i];  
    }
   break;

   case 29:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq30[i];  
    }
   break;

   case 30:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq31[i];  
    }
   break;

   case 31:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq32[i];  
    }
   break;

   case 32:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq33[i];  
    }
   break;

   case 33:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq34[i];  
    }
   break;

   case 34:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq35[i];  
    }
   break;

   case 35:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq36[i];  
    }
   break;

   case 36:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq37[i];  
    }
   break;

   case 37:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq38[i];  
    }
   break;

   case 38:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq39[i];  
    }
   break;

   case 39:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq40[i];  
    }
   break;

   case 40:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq41[i];  
    }
   break;

   case 41:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq42[i];  
    }
   break;

   case 42:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq43[i];  
    }
   break;

   case 43:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq44[i];  
    }
   break;


   case 44:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq45[i];  
    }
   break;


   case 45:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq46[i];  
    }
   break;


   case 46:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq47[i];  
    }
   break;

   case 47:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq48[i];  
    }
   break;


   case 48:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq49[i];  
    }
   break;

   case 49:
     for (i = 0; i < 20; i++) {
     mclock_ctr_loadseq[i] = sq50[i];  
    }
   break;


  }  
}

now you might say, "why not have the switches load a function instead?" however, i think that would just generate extra code since i still have to choose between one of 50 arrays (see this thread http://www.midibox.org/forum/index.php?topic=8902.0)

[audiocommander]

sneakthief,

if you look at your code, you can see that you might somehow do that in another way, 'cause every case results in a call to an array ("case# + 1"):

   case 24:

     for (i = 0; i < 20; i++) {

     mclock_ctr_loadseq = sq25; 

    }

   break;

   case 25:

     for (i = 0; i < 20; i++) {

     mclock_ctr_loadseq = sq48; 

    }

  break;

this pattern changes for case 25 ?

that may be an indicator why there's something going wrong.

But honestly: I guess you should either change your concept radically or listen to stryd's tipps in the previous topic (using pointers, duplicating the whole array or changing the linker to support larger multi-dimensional arrays). A 50 time switch with 20 time for loops is not exactly what I would call an elegant and simple solution :-\

Sorry I can't get you more detailed thoughts, but it's a complicated thing and requires a lot of time to develop a concept for timed sequencing.

Is there any reason why you cannot use TK's seq v2 or v3? What does your seq what the official one cannot?

Cheers,

Michael

[stryd_one]

Sneakthief's seq seems like it's a kind of (dual) linear phrase sequencer, so the seqv3 is a different beast :)

The multidimensional arrays are working OK provided that you don't reserve more space than required in the linker, so I would certainly recommend that.

I'd be interested to see the output of that switch statement....

[sneakthief]

A 50 time switch with 20 time for loops is not exactly what I would call an elegant and simple solution

no doubt - that's why i posted "newbie alert" and am looking for advice!

first of all, other than not being able to use switch case 25-28, my sequencer now works perfectly :) i can try some various workaround in the meantime.

re. mbseq - yes, TK's sequencer is amazing; however it can't handle long sequences (up to 256 measures) and has many unnecessary features and requirements for my specific needs.

re. timing - my loaded sequences only start playing at the beginning of the next measure. after extensive testing, the timing is actually perfect and there's no delay incurred by those switch loops.

re. bad case - ignore the seq48 typo. i was trying some different things out and accidentally left that in when i posted. it still crashes when it's sequentially ordered.

as i said, even when there's nothing in the cases, it still crashes, eg

   case 24:
   break;

re. changing the linker - i'm going to try that asap. (although i still had some weird data issues when using smaller multi-d arrays - hopefully it was just my crappy code ;) )

re. pointers - i'll try and wrap my head around these. i still haven't figured out how to use them here yet.

thanks for the feedback 8)

[stryd_one]

sneak, can you upload or PM me the ./output/(filename).asm file?

The answer may be in ASM land :(

[stryd_one]

Wow man... I see how your concept works but I tell ya what, SDCC really made a total dog's breakfast out of your code.

I would definitely look into pointers. Especially where that switch statement has the for loop on every case... In ASM that is not looking good. You might get away with the timings now but I suspect that when you scale the seq up to the full track count you may have some real problems :(

I'm curious... Do the sequences need to be const char? try declaring them as unsigned char, I noticed this:

       ustat_pic18f452_00      udata   0x000f80       data   0x000005
       ustat_pic18f452_01      udata   0x000f89       data   0x000005
       ustat_pic18f452_02      udata   0x000f92       data   0x000005
       ustat_pic18f452_03      udata   0x000f9d       data   0x000006
       ustat_pic18f452_04      udata   0x000fa6       data   0x000004
       ustat_pic18f452_05      udata   0x000fab       data   0x000005
       ustat_pic18f452_06      udata   0x000fb1       data   0x000003
       ustat_pic18f452_07      udata   0x000fba       data   0x000006
       ustat_pic18f452_08      udata   0x000fc1       data   0x000013
       ustat_pic18f452_09      udata   0x000fd5       data   0x00002b

Note the last one... fd5+2b = 0x1000. FFF is the last available ram.

ACCESSBANK NAME=accesssfr  START=0xF80          END=0xFFF          PROTECTED

You used one byte too many. There's plenty of ram free to just put them there.

GL!

[th0mas]

Without looking too deeply in the code I'd bet that your problem is arising with the ASM that SDCC is generating.  Basically there is probably some very low number of case blocks you can put in one switch/case block.  It'll probably have to do with the size of a variable used to calculate a jump table offset.  That's just a guess :D.

There's an easy solution.  Try breaking your code into two different switch/case blocks.  It shouldn't affect functionality really.  In other words break it like so:

switch (case) {

  case 1:

  case 2:

.

.

.

  case x:

}

switch (case) {

  case x+1:

  case x+2:

..

.

.

default:

}

It's probably still not a robust solution. but if it works, who cares :D - judging from your previous posts you will be playing it on stage while most of us argue about what is the best implementation in code :D.

[stryd_one]

Without looking too deeply in the code I'd bet that your problem is arising with the ASM that SDCC is generating.  Basically there is probably some very low number of case blocks you can put in one switch/case block.  It'll probably have to do with the size of a variable used to calculate a jump table offset.  That's just a guess :D.

great minds think alike. I looked at the ASM for just that and it didn't seem to go wrong, but when you posted, I double checked it to be sure, nd realised that I forgot to add the 1 to the case. That's exactly what happened. SDCC creates temporary variables to handle functions, called r0x00, r0x01, etc. in this case up to r0x37, and where it flips back to the first one, is where it goes awry (mclock.asm line 2686).

Just for comparison, the vX uses 37 of these variables too.... but that's for 5 files. The most in any file is 12, the least two. SDCC really made unclean code of that switch statement.

There's an easy solution.  Try breaking your code into two different switch/case blocks.  It shouldn't affect functionality really.  In other words break it like so:

It's probably still not a robust solution. but if it works, who cares :D - judging from your previous posts you will be playing it on stage while most of us argue about what is the best implementation in code :D.

LMAO you'll be right about that I swear... Although timing is almost surely going to become an issue I think. But the split-switch should work...

[sneakthief]

the split-switch didn't work. same error in the same place.

the arrays don't need to be constants - i'll try them as unsigned.

in addition to trying some of the good ideas presented here, this is my plan of action:

1. try stryd_one's linker for larger multi-d arrays

2. try adapting other suggestions i've received from people such as:

 	typedef struct song {
  char data[255];
} song;

static song *songs;

char songval(int songnumber, int position)
{
  return songs[songnumber].data[position];
}

and

char* whatSeq(int index)
{
/*.....*/
return &seq23;
}

from a function and that'll give you a pointer to the given array (a pointer is more or less the same as an array in C).

So then you can go:

char[] seq = whatSeq(5); 

[sneakthief]

I'm curious... Do the sequences need to be const char? try declaring them as unsigned char, I noticed this:

GL!

BTW unsigned chars don't work in this case:

"error: no target memory available for section ".idata"

it alright thought because i striped some unnecessary bits :)

[sneakthief]

update - re. the switch-case compiler error: i just noticed that the sdcc error is gone!

however, as i mentioned it still crashes at switch 25 & 26. weird.

[stryd_one]

Re: Any C Sequencer examples? Thread has a possible solution to this. If you compile the app now, and search the output file for the switch statement, you'll see a nice tidy table, and you may notice that there are now only 5 temp variables used, instead of the previous 37

The trick is, if you have a single statement in each case, then SDCC creates a jumptable, so it's nice and clean.