Re: [hatari-devel] Big update to cpu emulation pushed to main repo

[Thomas Huth <th.huth@xxxxxxxxx>]

Am Sun, 23 Feb 2020 20:05:09 +0200
schrieb Toni Wilen <twilen@xxxxxxxxxx>:

> >> This is really an incredible work, so thanks to Toni ;-)  
> > 
> > I want to do something that is done too rarely, since people usually
> > write to complain.
> > 
> > Thank you -- also to Toni, if he's reading this! This CPU update
> > really is impressive work and will for sure fix one or the other
> > emulation bug. I'm certain that the minor hitches reported here
> > will also be resolved.  
> 
> I am reading this, thanks :)

Also a big thank you from my side! Your hard work is really appreciated!

I've now had a try with the latest version of Hatari, and I get some
compiler warnings in the normal "release" builds, too - which currently
break my CI setups since I compile the sources with -Werror there, so
it would be great to get these fixed.

First one is harmless and easy to fix:

src/cpu/gencpu.c:1509:13: warning: 'fill_prefetch_next_skipopcode' defined but not used [-Wunused-function]
 1509 | static void fill_prefetch_next_skipopcode(void)
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.... that function is apparently not used anymore and thus could be
removed from the gencpu.cpp source now.


The second warnings look like a bug to me, though:

src/cpu/cpuemu_31.c: In function 'op_4cbb_31_ff':
src/cpu/cpuemu_31.c:21627:9: warning: 'tmppc' may be used uninitialized in this function [-Wmaybe-uninitialized]
21627 |  srca = x_get_disp_ea_020(tmppc, 0);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
src/cpu/cpuemu_31.c: In function 'op_4cfb_31_ff':
src/cpu/cpuemu_31.c:21885:9: warning: 'tmppc' may be used uninitialized in this function [-Wmaybe-uninitialized]
21885 |  srca = x_get_disp_ea_020(tmppc, 0);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~

The code in the function looks like this:

/* MVMEL.W #<data>.W,(d8,PC,Xn) */
uae_u32 REGPARAM2 CPUFUNC(op_4cbb_31)(uae_u32 opcode)
{
	int count_cycles = 0;
	uae_u32 dstreg = 3;
	OpcodeFamily = 37;
	CurrentInstrCycles = 12; 
	uae_u16 mask = get_iword_mmu040(2);
	uae_u32 dmask = mask & 0xff, amask = (mask >> 8) & 0xff;
	uaecptr tmppc;
	uaecptr srca;
	m68k_incpci(4);
	if (mmu040_movem) {
		srca = mmu040_movem_ea;
	} else
	tmppc = m68k_getpci();
	srca = x_get_disp_ea_020(tmppc, 0);
        ...
}

Thus the tmppc is really used without being initialized here, and the
"srca = x_get_disp_ea_020()" always overwrites the previous "srca =
mmu040_movem_ea;" ... which can't be right, can it?

Do you get the same generated code in WinUAE here, too, Toni?

 Best regards,
  Thomas