UAE4All 68020-emulation fix

[john4p]

Not long ago finkel made fullspeed emulation of many AGA games on Pandora possible with his own 68020-emulation.
It contains an error (probably tiny) in at least one of its calculation functions (he's suspecting MUL or DIV, see quote below).
The expectation of the AnUAE4All-developer cashing in on his work destroyed any motivation for him to fix this issue.
But now lubomyr has ported UAE4All 2.0 to Android which eliminated the potential threat of anyone earning money from finkel's work since lubomyr is not going to sell it for money.
The bug makes Slam Tilt and Roadkill unplayable (and other games as well). If it was fixed I'm sure they'd fully work.

Here's a report of the bug and finkel's response from the Pandora boards:

Originally Posted by anonymous

Confirmed on both accounts. These "non-critical" bugs are the hardest to find, but I do have some ideas where to start. I was using WinUAE 1.45 source to help me debug 68020 code in UAE4ALL, so it should be possible to find which 68020 instructions are causing problems. Since those score numbers are so ridiculous, I'm suspecting MUL or DIV. The same problem may be responsible for Roadkill as well.

I haven't worked on UAE4ALL for over a year, though. I'll see if I can find some spare time to get back to it, but knowing that someone is trying to 'score' (pardon my pun) on my work certainly doesn't motivate me in any way.

Thanks for the offer to donate pandora, but money's one thing I don't have too much problems with. I'd like it much more if that matter with anUAE4ALL source was resolved.[...]

Since finkel seems to be gone could you please have a look at the attached famec_opcodes.h if you can find the MUL- or DIV-error(s)?

[Toni Wilen]

Attachment is mostly useless because all logic is in other files.

I quickly tested RoadKill AGA and it does use MULL (opcodes 0x4c00 and 0x4c3c)

[john4p]

Thanks for the quick reply! I'll have a look tomorrow morning and upload the main logic to this.

[john4p]

EDIT: Disregard this post - it's nonsense.

I've now attached the famec.cpp which #includes the famec_opcodes.h.

Originally Posted by Toni Wilen

I quickly tested RoadKill AGA and it does use MULL (opcodes 0x4c00 and 0x4c3c)

Both opcodes are missing in the famec_opcodes.h. It only contains MULS and MULU opcodes.

Here's one of the MULU opcodes in famec_opcodes.h:

/* MULU */
OPCODE(0xC0C0)
{
    u32 res;
    u32 src;

    src = DREGu16((Opcode >> 0) & 7);
    res = DREGu16((Opcode >> 9) & 7);
    res *= src;
    flag_N = res >> 24;
    flag_NotZ = res;
    flag_V = flag_C = 0;
    DREGu32((Opcode >> 9) & 7) = res;
#ifdef FAME_ACCURATE_TIMING
    /* count bits set in the memory operand */
    io_cycle_counter -= bitset_count(src & 0xFFFF) * 2;
    RET(38)
#else
    RET(16 + 38)
#endif
}

And here is one of the MULS opcodes:

/* MULS */
OPCODE(0xC1C0)
{
    u32 res;
    u32 src;

    src = (s32)DREGs16((Opcode >> 0) & 7);
    res = (s32)DREGs16((Opcode >> 9) & 7);
    res = ((s32)res) * ((s32)src);
    flag_N = res >> 24;
    flag_NotZ = res;
    flag_V = flag_C = 0;
    DREGu32((Opcode >> 9) & 7) = res;
#ifdef FAME_ACCURATE_TIMING
    /* count bits set in the memory operand */
    io_cycle_counter -= bitset_count(src ^ (src << 1)) * 2;
    RET(38)
#else
    RET(8 + 38)
#endif
}

Can you create the implementation/definition for the opcodes 0x4c00 and 0x4c3c out of this and the code in famec.cpp?

(I'll test what happens if I just use the same code for MULL as MULU.)

[Toni Wilen]

They are not missing. Search using opcode values.

[john4p]

Sorry - the post above is completely wrong: I've searched in the famec_opcodes.h from the old uae4all1.

Now in finkel's 68020-including famec_opcodes.h I've found that opcodes 0x4c00 and 0x4c3c are defined:

// MULS/MULU.L Dn - Long Format, 68020+
OPCODE(0x4C00)
{
   u32 adr, res;
   u32 src, dst;

#ifdef M68020_000_ILLEGAL_INTRUCTION_EX
   if (prefs_cpu_model < M68020)
      EXECUTE_EXCEPTION(M68K_ILLEGAL_INSTRUCTION_EX)
#endif
      
   FETCH_WORD(res)
   src = DREGu32((Opcode /*>> 0*/) & 7);
   
   MULL(src, res);
   
#ifdef USE_CYCLONE_TIMING
RET(54)
#else
RET(50)
#endif
}

[...]

// MULS/MULU.L <xx>.W, <xx>.L - Long Format, 68020+
OPCODE(0x4C3C)
{
   u32 adr, res;
   u32 src, dst;

#ifdef M68020_000_ILLEGAL_INTRUCTION_EX
   if (prefs_cpu_model < M68020)
      EXECUTE_EXCEPTION(M68K_ILLEGAL_INSTRUCTION_EX)
#endif
   
   FETCH_WORD(res)
   FETCH_LONG(src)
   
   MULL(src, res);
   
#ifdef USE_CYCLONE_TIMING
RET(54)
#else
RET(50)
#endif
}

I've attached the correct famec.cpp now which implements MULL():

static void MULL(u32 src, u16 extra)
{
   if (extra & 0x800) {
      /* signed variant */
      s64 a;
      
      a = (s32)src * (s32)DREG((extra >> 12) /*& 7*/);
      flag_V = 0;
      flag_C = 0;
      flag_NotZ = (a != 0);
      if (extra & 0x400) {
         /* 32 x 32 -> 64 */
         DREG(extra & 7) = (u32)(a >> 32);
         flag_N = (a >> 56);
      }
      else if ((a & UVAL64 (0xffffffff80000000)) != 0
            && (a & UVAL64 (0xffffffff80000000)) != UVAL64 (0xffffffff80000000))
      {
         flag_V = M68K_SR_V;
      }
      else
         /* 32 x 32 -> 32 */
         flag_N = (a >> 24);
      DREG((extra >> 12) /*& 7*/) = (u32)a;
   } else {
      /* unsigned */
      u64 a;
      
      a = src * DREG((extra >> 12) /*& 7*/);
      flag_V = 0;
      flag_C = 0;
      flag_NotZ = (a != 0);
      flag_N = (a >> 56);
      if (extra & 0x400) {
         /* 32 x 32 -> 64 */
         DREG(extra & 7) = (u32)(a >> 32);
         flag_N = (a >> 56);
      }
      else if ((a & UVAL64 (0xffffffff00000000)) != 0) {
         flag_V = M68K_SR_V;
      }
      else
         /* 32 x 32 -> 32 */
         flag_N = (a >> 24);
      DREG((extra >> 12) /*& 7*/) = (u32)a;
   }
}

Can you see an error in the implementation?

[Toni Wilen]

a = src * DREG((extra >> 12) /*& 7*/);

I think this creates 32-bit result, not 64-bit because casts are missing. (C will do 32-bitx32-bit=32-bit multiplication first then converts the result to 64-bit, truncating upper 32-bits)

a = (u64)src * DREG(...

should fix it.

Same bug in signed multiplication. Compare with UAE code, it is nearly identical.

[john4p]

That was fast - thanks! I'll try this.

[john4p]

Roadkill works now - thanks a bunch, Toni!

Unfortunately in Slam Tilt the score counter is still messed up. Only shows "00", "909,090,909,090", "404,040,404,040" or "202,020,202,020".

I'll try to change DIVL, too, to match its WinUAE-counterpart. Maybe that'll help.

[john4p]

Changed everything in MULL and DIVL to be exactly as in WinUAE. Sadly this didn't fix Slam Tilt's score counter (btw., I've now seen every "x0x,0x0,x0x,0x0"-combination with x being a single digit > 0).

Any idea what could cause this problem?

[Toni Wilen]

Slam Tilt uses both MULL and DIVL (div by 1000000, mul by 1000000, div by 100000 and so on.. Not very optimal way to do integer to number string conversion).

I don't see anything obviously wrong in attached DIVL code.

[john4p]

Okay, thanks a lot for your checking and the informations. If the MULL and DIVL code is correct now maybe the definition of one of the opcodes in famec_opcodes.h which uses MULL and/or DIVL is wrong?

[john4p]

I've now also tried the code for MULL and DIVL which only uses u32 and s32 variables (no u64 and s64), but that didn't fix the Slam Tilt issue, either. So I'm sure the functions MULL and DIVL are correct now.

Looked which mul-&div-opcodes Slam Tilt utilizes during gameplay. They were 0x4c00, 0x4c18, 0x4c28, 0x4c40, 0x4c50, 0x4c3c and 0x4c7c.
Since Roadkill works correctly we can probably exclude 0x4c00 and 0x4c3c for being erroneous.

Here are now the remaining five opcode-definitions in finkel's improved fame/c-version:

// MULS/MULU.L (An)+ - Long Format, 68020+
OPCODE(0x4C18)
{
   u32 adr, res;
   u32 src, dst;
   
   FETCH_WORD(res)
   adr = AREG((Opcode /*>> 0*/) & 7);
   AREG((Opcode /*>> 0*/) & 7) += 4;
   
   PRE_IO
   READ_LONG_F(adr, src)
   POST_IO
   MULL(src, res);
   
   RET(50)
}

// MULS/MULU.L (d16,An) - Long Format, 68020+
OPCODE(0x4C28)
{
   u32 adr, res;
   u32 src, dst;

   FETCH_WORD(res)
   FETCH_SWORD(adr)
   adr += AREG((Opcode /*>> 0*/) & 7);
   
   PRE_IO
   READ_LONG_F(adr, src)
   POST_IO
   MULL(src, res);

   RET(50)
}

// DIVSL/DIVUL.L Dn - Long Format, 68020+
OPCODE(0x4C40)
{
    u32 adr, res;
    u32 src, dst;
    u8 areg;

    FETCH_WORD(res);
    src = DREGu32((Opcode /*>> 0*/) & 7);
    if (src == 0)
    {
        execute_exception(M68K_ZERO_DIVIDE_EX);
        RET(10)
    }
    
    DIVL(src, res);
    
    RET(90)
}

// DIVSL/DIVUL.L (An) - Long Format, 68020+
OPCODE(0x4C50)
{
   u32 adr, res;
   u32 src, dst;

   FETCH_WORD(res)
   adr = AREG((Opcode /*>> 0*/) & 7);
   
   PRE_IO
   READ_LONG_F(adr, src)
   POST_IO
   if (src == 0)
   {
       execute_exception(M68K_ZERO_DIVIDE_EX);
       RET(10)
   }
   
   DIVL(src, res);
   
   RET(50)
}

// DIVSL/DIVUL.L <xx>.W / <xx>.L - Long Format, 68020+
OPCODE(0x4C7C)
{
   u32 adr, res;
   u32 src, dst;

   FETCH_WORD(res)
   FETCH_LONG(src)
   
   if (src == 0)
   {
       execute_exception(M68K_ZERO_DIVIDE_EX);
       RET(10)
   }
   
   DIVL(src, res);
   
   RET(50)
}

Btw., I tried enabling the commented /*>> 0*/-parts but that also didn't fix the bug.


Here are the defines of the macros used (from famec.cpp):

#define READ_LONG_F(A, D)           \
		D = Read_Word((A)) & 0xFFFF;   \
		D |= Read_Word((A) + 2) << 16;

#define FETCH_WORD(A)       \
		(A) = *PC++;

#define FETCH_SWORD(A)      \
		(A) = (s32)(s16)(*PC++);

#define FETCH_LONG(A)                        \
      (A) = PC[1] | (PC[0] << 16);              \
      PC += 2;

#define RET(A)                                      \
    io_cycle_counter -= (A);                        \
    if (io_cycle_counter <= 0) goto famec_Exec_End;	\
    goto famec_Exec;

PRE_IO and POST_IO do nothing (what they used to do is commented).

Also u32, s32, u64 and s64 are defined as follows in famec.cpp (I use g++ 4.3.3 to build on the Pandora which has an ARM Cortex A8 CPU):

#define u32 unsigned int
#define s32 signed int
#define u64 unsigned long long
#define s64 long long

Can you please have a look if one of the opcode-definitions contains an error?

[Toni Wilen]

Looks ok.

SlamTilt uses also BFEXTU and BFINS. (68020+ bit field instructions, rarely used) Very messy code, difficult to check..

[john4p]

Thanks again for your repeated examination of the code and the additional info.
I'll do some random trial&error manipulations of BFEXTU and BFINS tomorrow.
If that doesn't help then I'll just accept that I have to play Slam Tilt on the laptop.

[john4p]

The "random trial&error manipulations" didn't lead to any success.
Found that out of the BFEXTU- and BFINS-opcodes Slam Tilt seems to only use 0xe9e8 (BFEXTU (d16,An) - 68020+) and 0xefc0 (BFINS Dn - 68020+).
When I messed with 0xe9e8 the table loaded but the screen turned grey as soon as I pressed F1 to start a game.
When I changed something in 0xefc0 (like removing the comment from BF_MASK) the table also loaded but when I pressed F1 the ball instantly dropped off the screen (just fell through the bottom) before I could push it into play.

Please have a final look at the two opcode definitions. Maybe you see an ovious error:

// BFEXTU (d16,An) - 68020+
OPCODE(0xE9E8)
{
   u32 adr, res;
   u32 src, dst;
   u32 mask, width;
   s32 offset;
   u32 bf0, bf1;

   FETCH_WORD(res)
   FETCH_SWORD(adr)
   adr += AREG((Opcode /*>> 0*/) & 7);
   BF_GET_PARM(res, offset, width)
   BF_MEM_GET(&adr, &dst, &offset, width, &bf0, &bf1);
   BF_SET_FLAGS(dst, width)
   DREG((res >> 12) /*& 7*/) = dst;
RET(12)
}

// BFINS Dn - 68020+
OPCODE(0xEFC0)
{
   u32 adr, res;
   u32 src, dst;
   u32 mask, offset, width;
   
   FETCH_WORD(res)
   dst = DREG((Opcode /*>> 0*/) & 7);
   src = DREG((res >> 12) /*& 7*/);
   BF_GET_PARM(res, offset, width)
   offset &= 0x1F;
   BF_SET_FLAGS(src, width)
   mask = ((u32)0xFFFFFFFF << (32 - width)) >> offset;
//   BF_MASK(mask, offset, width)
//   BF_SHIFT_UP(mask, offset, width)
   BF_SHIFT_UP(src, offset, width)
   DREG((Opcode /*>> 0*/) & 7) = (dst & ~mask) | (src & mask); 
RET(8)
}

Well, maybe there isn't anything to find there because there's no bug and these instructions have nothing to do with the Slam Tilt score counter bug at all...

[john4p]

The Slam Tilt issue has finally been resolved: TomB found out that an "incremented register was not written back to m68kcontext in UNPK" and fixed this.