Work on the Minimig core?

[arty]

Here's a build with the MUL and DIV fixes. Works very well for me.

Great. Thank you!

[retrofun]

Here are some changes/fixes I'd like to propose/discuss:

• TG68K: limit address space and Fast RAM to 24bit for 68000/68010
  https://github.com/retrofun/minimig-mis ... 17d3048f9d
  
  The external address bus of 68000/68010 is 24bit. Currently TG68K uses 32bit.
  
  With address bus limited to 24bit (like on the real processors) there are some more things to fix:
  • ABCD.B, NBCD.B, SBCD.B tests fail with V bit issues. cputest expects other behaviour.
  • UNLK.L test fails
  This also limits the max. Fast RAM to 8MB for 68000/68010 (like on real AMiGAs).
  
  Corresponding commit for the mist firmware to show 8MB (68000/68010) / 24MB (68020) in the OSD menu:
  
  minimig-mist: max. memory 8MB (68000/68010), 24MB (68020)
  https://github.com/retrofun/mist-firmwa ... a3d3f3f38d

• TG68K: fix ILLEGAL.B for b0000... opcode group
  https://github.com/retrofun/minimig-mis ... 529ea8f880
  
  Filters illegal ea addressing modes.
  This pushes the ILLEGAL.B tests from 0 to 1440.
  
  Still 15 opcode groups to do.

• TG68K: mask out unused bits for SR in ORI and EORI (68000/68010)
  https://github.com/retrofun/minimig-mis ... 83e9cc525b
  
  Masks out unused SR bits T0 & M of system byte for 68000/68010.
  Follow up commit for https://github.com/mist-devel/minimig-m ... ba8fa78e29
  
  This pushes the 68000 cputests
  • EORSR.W from 16386 to 131074
  • ORSR.W from 16386 to 131074

[slingshot]

Wow, lots of work to filter out illegal instructions.
I wonder if the SR masking could be more general (just adding the masking to the end of the block for SRin, instead of to the OR and XOR case), probably move to SR also affected.

[apolkosnik]

Would it be better to mask out the SR in a spot where it gets written after modification? Just below: "if exec(to_SR) = '1' then".
That would also take care of move instruction.

[apolkosnik]

Wow, lots of work to filter out illegal instructions.
I wonder if the SR masking could be more general (just adding the masking to the end of the block for SRin, instead of to the OR and XOR case), probably move to SR also affected.

That's what I did for Mister. https://github.com/MiSTer-devel/Minimig ... a3d79a37e4

[retrofun]

Wow, lots of work to filter out illegal instructions.
I wonder if the SR masking could be more general (just adding the masking to the end of the block for SRin, instead of to the OR and XOR case), probably move to SR also affected.

Yep, this pushes 68020/MV2SR.W cputest from 818 to 1034.

[ijor]

Would it be better to mask out the SR in a spot where it gets written after modification? Just below: "if exec(to_SR) = '1' then".
That would also take care of move instruction.

The unused SR bits, at least on the 68000, simply don't exist at the hardware. Then, IMHO, the correct way is actually don't define them at all. Yes, this might make part of the code slightly less straightforward, and if you never use the registers, the compiler will optimize out them anyway.

But I think it is a good idea to write the code more similar to how the hardware works. And personally, I like to have all the code that modifies registers together. It might be an issue of personal style. But it helps to make the implementation more accurate instead of patching and patching when something doesn't work and you don't know exactly why. As you can imagine, at the hardware logic level, there even isn't such a thing as SR. There is no relation between the CCR bits, the interrupt mask and the other bits. They are actually physically located separately (at the die layout) as they belong to completely different sections. They are together at the programmer level just for convenience.

Again, it is perfectly possible to implement this differently. But sooner or later you might get into troubles. Say, what if the unused bits are set and not cleared in some cases. I can tell you that this doesn't happen in this case. The unused bits are always written as zero (again, at least at the 68000). But other cases are more complex.

[apolkosnik]

Would it be better to mask out the SR in a spot where it gets written after modification? Just below: "if exec(to_SR) = '1' then".
That would also take care of move instruction.

The unused SR bits, at least on the 68000, simply don't exist at the hardware. Then, IMHO, the correct way is actually don't define them at all. Yes, this might make part of the code slightly less straightforward, and if you never use the registers, the compiler will optimize out them anyway.

But I think it is a good idea to write the code more similar to how the hardware works. And personally, I like to have all the code that modifies registers together. It might be an issue of personal style. But it helps to make the implementation more accurate instead of patching and patching when something doesn't work and you don't know exactly why. As you can imagine, at the hardware logic level, there even isn't such a thing as SR. There is no relation between the CCR bits, the interrupt mask and the other bits. They are actually physically located separately (at the die layout) as they belong to completely different sections. They are together at the programmer level just for convenience.

Again, it is perfectly possible to implement this differently. But sooner or later you might get into troubles. Say, what if the unused bits are set and not cleared in some cases. I can tell you that this doesn't happen in this case. The unused bits are always written as zero (again, at least at the 68000). But other cases are more complex.

So, my point was to do the filtering out at the single spot where the SR gets written rather than filtering it out for every instruction touching Status Register e.g.
https://github.com/MiSTer-devel/Minimig ... 060505ec10

[ijor]

So, my point was to do the filtering out at the single spot where the SR gets written rather than filtering it out for every instruction touching Status Register e.g.
https://github.com/MiSTer-devel/Minimig ... 060505ec10

That's ok. My comment was meant to be more generic about the issue than replying to you, about your specific implementation (which, honestly, I didn't check, sorry).

Note that the 68000 actually never writes directly to the SR. All modifications to SR originated from instructions that specifically modify the SR, go through an intermediate register. This has some subtle implications if you aim for cycle accuracy.

[ijor]

One thing i have noticed is that if i rte from the illegal trap then the illegal instruction is executed again (and again ....). Is this correct behaviour?

It is. The reason is that the illegal instruction is actually never executed. There is no microcode associated with illegal instructions. The exception is triggered before. So the PC still points to the illegal instruction after the exception.

What I've also noticed: There's no address error at all in tg68k, not even in 68k mode. It just accesses odd addresses. But can address errors ever happen on a 68020?

Yes, but only during instruction prefetch.

[tobiflex]

I like to say hello here:
HELLO!

I have done my version of the TG68K.C here:
https://opencores.org/projects/tg68kc

I have change the license in the past from GPL to LGPL.
I'm also try to fix the bugs there. I'm very happy for the creation of the cputester from Toni Wilen.

[slingshot]

Here are some changes/fixes I'd like to propose/discuss:

• TG68K: limit address space and Fast RAM to 24bit for 68000/68010
  https://github.com/retrofun/minimig-mis ... 17d3048f9d
  
  The external address bus of 68000/68010 is 24bit. Currently TG68K uses 32bit.
  
  Corresponding commit for the mist firmware to show 8MB (68000/68010) / 24MB (68020) in the OSD menu:
  
  minimig-mist: max. memory 8MB (68000/68010), 24MB (68020)
  https://github.com/retrofun/mist-firmwa ... a3d3f3f38d

Can you send a PR with the firmware change?

[slingshot]

[Yep, this pushes 68020/MV2SR.W cputest from 818 to 1034.

I've changed then:
https://github.com/mist-devel/minimig-m ... 0e507a65e4

[tobiflex]

Hovewer this is a real issue:
d0=0x10
d1=0
d5=80008080

divsl.l d0,d1:d5
d5 - negative, d0 - positive, result -> negative
expected SR -> 0408 (N=1), result -> 0400 (N=0)

try changing
https://github.com/mist-devel/minimig-m ... U.vhd#L766
to

Code: Select all

  Flags(3 downto 0) <= set_flags(3) & flag_z(1) & "00";

I would actually expect this to have a negative impact on other div variants but i haven't found one

This shut be so:

Code: Select all

  Flags(3 downto 0) <= set_flags(3) & set_flags(2) & "00";

This correct also the Z-Flag for DIVL.

[tobiflex]

In 2017 i had rework the BCD-Opcodes. But cputester stop here.
If i set V-Flag <= '0' pass cputester.
But i have compare my version with a real 68000 - whats wrong???

reworked BCD-Opcodes inside this version:
https://opencores.org/projects/tg68kc

There is also a generic for a hardware multiplier.

Sorry for my bad english...

[DanyPPC]

I will test new core.

[DanyPPC]

New Core has problems. Pinball Illusions vertical scrolling doesn't work as due.

[vebxenon]

New Core has problems. Pinball Illusions vertical scrolling doesn't work as due.

Same here...

[slingshot]

try changing
https://github.com/mist-devel/minimig-m ... U.vhd#L766
to

Code: Select all

  Flags(3 downto 0) <= set_flags(3) & flag_z(1) & "00";

I would actually expect this to have a negative impact on other div variants but i haven't found one

The Pinball Illusions game found it

[kolla]

Haha, I just came to report about Pinball Illusions, lol.

On the bright side, IBrowse 2.5 keyfile validator now works (though it was reworked in 2.5.1 to avoid MULU).

[slingshot]

This shut be so:

Code: Select all

  Flags(3 downto 0) <= set_flags(3) & set_flags(2) & "00";

This correct also the Z-Flag for DIVL.

Pinball Illusions still glitching with this. Something wrong with the N-Flag (the original one works with the game, but fails in some testcases - weird).

[apolkosnik]

@tobiflex, thank you for creating the tg68k core, and making it available for everyone to play with!

The latest version of WinUAE cputester still has some issues with the flags on DIVU and DIVS, please see Toni's post:
http://eab.abime.net/showpost.php?p=135 ... stcount=23

[DanyPPC]

Tested latest core with various game, no problems

Right way !

[tobiflex]

if this code is executed:

Code: Select all

	moveq	#-1,d3
	moveq	#32,d0
	lsl.l	d0,d3
	asr.w	#2,d3
	bvs.s	fail

Is the V-Flag set, this is wrong

bugfix - change line 740 in TG68K_ALU.vhd from

Code: Select all

		if exec(opcROT) = '1' then
		  asl_VFlag <= ((set_flags(3) xor rot_rot) OR asl_VFlag);
		else
		  asl_VFlag <= '0';
		end if;

to:

Code: Select all

		IF exec(opcROT)='1' AND decodeOPC='0' THEN
			asl_VFlag <= ((set_flags(3) XOR rot_rot) OR asl_VFlag);	
		ELSE	
			asl_VFlag <= '0';
		END IF;	

[slingshot]

I think I could decrypt the DIV errors:
DIVS.W:
divs.w d1,d0
d1 = 0
d0= 0x10
Exception ID: expected 5 but got no exception.

Hmm. I see the exception happen. I'd be very surprised if such a fundamental error still exists.

I've debugged this a bit, and the real problem is that Div by zero exception should create stack frame format #2 on 68020. But that's not implemented, and creates format #0.
That's also true for Trace exceptions, maybe that's why there are many failures connected to the Trace flags.