Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

68040 Takes Wrong Branch of If Else

Any good 68k assembly programmers out there?? I'm using a commercial Green Hills compiler for a Motorola 68040 and I'm seeing some very strange behavior from the code. Sometimes, the code will do an if/else comparison, and take the wrong branch. For example:

float a = 1, b = 2;

if (a < b)
    do c;
else 
    do d;

The code will sometimes to d!? I've found that whenvever this error occurs, there is always one particular ISR that interrupts the comparison. I took a look at the generated assembly for the ISR and saw a few things that don't make sense to me. First it looks like the floating point status registers, FPSR, FPCR, and FPIAR, are not saved in the ISR. This would explain why the if/elses are taking the wrong branch. The FPSR register is used to determine the result of a comparison, and if that register is overwritten in an ISR then the branch may take the wrong path. Following is the entry and exit assembly generated by the compiler:

isr_function:
    FSAVE   -(%SP)
    LINK    %A6,#-192
    MOVEM.L %D0/%D1/%D2/%A0/%A1,-(%SP)
    FMOVEM  %FP0/%FP1/%FP2/%FP3/%FP4/%FP5/%FP6/%FP7,-(%SP)

    ; isr code ...

    FMOVEM  -308(%A6),%FP0/%FP1/%FP2/%FP3/%FP4/%FP5/%FP6/%FP7
    MOVEM.L -212(%A6),%D0/%D1/%D2/%A0/%A1
    UNLK    %A6
    FRESTORE    (%SP)+
    RTE

I looked through the Programmer's Reference Manual and I can't find anything that suggests that FSAVE, or FMOVEM, saves the FP status registers. Actually, I saw one comment that suggests that it does not, "The FSAVE does not save the programmerís model registers of the floating-point unit; it saves only the user invisible portion of the machine." So I added some assembly of my own to save away the registers at the start of the ISR, and restore them at the end, and this dramatically improved the performance, but I'm still seeing some problems. Following are the additions I made; the backup variables are typed as unsigned long in the C code:

isr_function:
    FSAVE   -(%SP)
    LINK    %A6,#-192
    MOVEM.L %D0/%D1/%D2/%A0/%A1,-(%SP)
    FMOVEM  %FP0/%FP1/%FP2/%FP3/%FP4/%FP5/%FP6/%FP7,-(%SP)

    FMOVE %FPIAR,fpiar_backup
    FMOVE %FPSR,fpsr_backup
    FMOVE %FPCR,fpcr_backup

    ; isr code ...

    FMOVE fpiar_backup,%FPIAR
    FMOVE fpsr_backup,%FPSR
    FMOVE fpcr_backup,%FPCR

    FMOVEM  -308(%A6),%FP0/%FP1/%FP2/%FP3/%FP4/%FP5/%FP6/%FP7
    MOVEM.L -212(%A6),%D0/%D1/%D2/%A0/%A1
    UNLK    %A6
    FRESTORE    (%SP)+
    RTE

I had a hard time believing the compiler actually had a bug by not saving the registers. So I started looking at the values of FPx and Dx to see that they are restored to the proper value, and it looks like they are not. However I'm not 100% that I'm not tainting the assembly code with my modifications. Following is the code I added to save the registers; the debug variables are typed as unsigned longs:

isr_function:
    FMOVE   %FP0,debug3
    FMOVE   %FP1,debug5
    FMOVE   %FP2,debug7
    FMOVE   %FP3,debug9
    FMOVE   %FP4,debug11
    FMOVE   %FP5,debug13
    FMOVE   %FP6,debug15
    FMOVE   %FP7,debug17
    FMOVE   %FPCR,debug19
    FMOVE   %FPIAR,debug23
    FMOVE   %FPSR,debug25   

    FSAVE   -(%SP)
    LINK    %A6,#-192
    MOVEM.L %D0/%D1/%D2/%A0/%A1,-(%SP)
    FMOVEM  %FP0/%FP1/%FP2/%FP3/%FP4/%FP5/%FP6/%FP7,-(%SP)

    ; isr code ...

    FMOVEM  -308(%A6),%FP0/%FP1/%FP2/%FP3/%FP4/%FP5/%FP6/%FP7
    MOVEM.L -212(%A6),%D0/%D1/%D2/%A0/%A1
    UNLK    %A6

    FMOVE   %FP0,debug4
    FMOVE   %FP1,debug6
    FMOVE   %FP2,debug8
    FMOVE   %FP3,debug10
    FMOVE   %FP4,debug12
    FMOVE   %FP5,debug14
    FMOVE   %FP6,debug16
    FMOVE   %FP7,debug18
    FMOVE   %FPCR,debug20
    FMOVE   %FPIAR,debug24
    FMOVE   %FPSR,debug26

    FRESTORE    (%SP)+
    RTE

In short my questions are,

1) is there a problem with the generated assembly in that it does not save the FPSR, FPCR, and FPIAR registers, and

2) am I properly saving the values of the registers when I enter and exit the ISR?

It would be great if I had another compiler to compare against. Unfortunately I'm unable to attach a debugger to the code. I have plenty of experience in C/C++/C#/Java/Python/PHP/etc., but I am far from an assembly expert.

Any ideas are appreciated!

like image 782
Samuel Avatar asked Nov 14 '11 05:11

Samuel


2 Answers

For future reference, the problem was indeed related to the compiler not saving the value of floating point status registers. I contacted Green Hills and according to them this is not a bug, and that saving the value of the registers is the responsibility of the programmer. Which is odd to me, because the compiler saves all of the other internal registers, including the internal state of the FPU, why stop with the status registers??

In short, saving the value of the FPSR and FPIAR coming into, and when leaving the ISR will correct the problem. The following should do the trick:

void isr(void)
{
    // variable declarations ...

    __asm("    FMOVE %FPIAR,-(%SP)"); 
    __asm("    FMOVE %FPSR,-(%SP)"); 

    // some code ...


    __asm("    FMOVE (%SP)+,%FPSR"); 
    __asm("    FMOVE (%SP)+,%FPIAR");
}
like image 176
Samuel Avatar answered Nov 02 '22 08:11

Samuel


I haven't done 68K programming since the days of the 68020 but I'll try to reach down into the relevant gray matter and/or web resources :-)

Answering your specific questions:

Is there a problem with the generated assembly in that it does not save the FPSR, FPCR, and FPIAR registers?

I would say yes, but only if there's something in the ISR that affects them. While this seems unlikely (ISRs are supposed to be fast so I wouldn't expect them to be mucking about with floating point stuff), prudence would seem to suggest that a routine save everything just on the off-chance that code may change it.

Having said that, I'm not sure how you're compiling the ISR (or even whether it's your code at all). It may be that a special flag is necessary to get the compiler to generate more code for saving other things.

Am I properly saving the values of the registers when I enter and exit the ISR?

Again, this depends. It looks okay but I'd be a little worried about using specific memory locations like fpiar_backup or debug26 unless you're very sure that the ISR itself isn't prone to another interrupt.

If the interrupts are disabled during ISR processing then you're probably okay.

In addition, it depends on what that ISR is servicing. The docs seem to indicate that any ISRs servicing floating point problems should always do the fsave first.

It would be helpful if you dumped out the values of those debugX locations so you could see what values are different between entry and exit from the ISR. And make sure they're the right size. And be careful that you're not looking at them in the middle of the ISR, where they'll almost certainly be different.

like image 2
paxdiablo Avatar answered Nov 02 '22 06:11

paxdiablo