Conflicting types in gen_input.h + some more porting issues

[GoogleCodeExporter]

Hi there ekeeke31,

I am one of the developers of Genesis Plus GX PS3, a fork of Genesis Plus GX. 
One of the main aims of the project is to keep the codebase as close to the 
original Genesis Plus GX codebase as possible.

In the future, I would hope to be able to pull in new revisions with as minimal 
an amount of manual patching as possible.

While porting to PS3, I came across a few issues -

=== gen_input.c / gen_input.h ===
On PS3, there are basically two compilers to choose from - SNC or GCC. When 
trying to compile gen_input.c, it would complain about conflicting types in the 
functions:

[quote]
extern void mouse_write(uint32 data);
extern uint32 mouse_read(void);
extern uint32 menacer_read (void);
extern uint32 justifier_read (void);
extern uint32 gamepad_1_read (void);
extern uint32 gamepad_2_read (void);
extern void gamepad_1_write (uint32 data);
extern void gamepad_2_write (uint32 data);
extern uint32 wayplay_1_read (void);
extern uint32 wayplay_2_read (void);
extern void wayplay_1_write (uint32 data);
extern void wayplay_2_write (uint32 data);
extern uint32 teamplayer_1_read (void);
extern uint32 teamplayer_2_read (void);
extern void teamplayer_1_write (uint32 data);
extern void teamplayer_2_write (uint32 data);
extern uint32 jcart_read(uint32 address);
extern void jcart_write(uint32 address, uint32 data);
[/quote]

The problem here is that in gen_input.c's function header signatures, they 
either have returntypes of unsigned ints or the arguments are of type unsigned 
int. When externing them in gen_input.h, however, they are suddenly uint32. GCC 
won't accept this.

SNC will compile this without any warnings or errors - however, SNC isn't 
available on firmwares lower than a certain arbitrary version, and I'd like to 
support as much as possible.

The way I'm currently working around this is that I set a flag in the Makefile 
when either compiling with the SNC or GCC compiler, and then do this within 
gen_input.h:

[quote]
/* Peripherals specific */
#ifdef GCC_COMPILER
extern void mouse_write(unsigned int data);
extern unsigned int mouse_read(void);
extern unsigned int menacer_read (void);
extern unsigned int justifier_read (void);
extern unsigned int gamepad_1_read (void);
extern unsigned int gamepad_2_read (void);
extern void gamepad_1_write (unsigned int data);
extern void gamepad_2_write (unsigned int data);
extern unsigned int wayplay_1_read (void);
extern unsigned int wayplay_2_read (void);
extern void wayplay_1_write (unsigned int data);
extern void wayplay_2_write (unsigned int data);
extern unsigned int teamplayer_1_read (void);
extern unsigned int teamplayer_2_read (void);
extern void teamplayer_1_write (unsigned int data);
extern void teamplayer_2_write (unsigned int data);
extern unsigned int jcart_read(unsigned int address);
extern void jcart_write(unsigned int address, unsigned int data);
#else
extern void mouse_write(uint32 data);
extern uint32 mouse_read(void);
extern uint32 menacer_read (void);
extern uint32 justifier_read (void);
extern uint32 gamepad_1_read (void);
extern uint32 gamepad_2_read (void);
extern void gamepad_1_write (uint32 data);
extern void gamepad_2_write (uint32 data);
extern uint32 wayplay_1_read (void);
extern uint32 wayplay_2_read (void);
extern void wayplay_1_write (uint32 data);
extern void wayplay_2_write (uint32 data);
extern uint32 teamplayer_1_read (void);
extern uint32 teamplayer_2_read (void);
extern void teamplayer_1_write (uint32 data);
extern void teamplayer_2_write (uint32 data);
extern uint32 jcart_read(uint32 address);
extern void jcart_write(uint32 address, uint32 data);
#endif
[/quote]

However, this is quite ugly and I'm sure this will be unmaintainable in the 
long run.

I think the best solution here (and the cleanest overall) would be to simply 
change all instances of uint32 here to unsigned int, as uint32 != unsigned int.

=== genesis.c ===

As a side note - I also encountered another problem to do with revision r496 - 
all of a sudden, ROMs would take a very long time to load.

I isolated what line in the revision was causing it - in genesis.c (on line 
141/142 - in the function gen_hardreset):

https://code.google.com/p/genplus-gx/source/diff?spec=svn496&r=496&format=side&p
ath=/trunk/source/genesis.c

[quote]
mcycles_68k = mcycles_z80 = (uint32)((MCYCLES_PER_LINE * lines_per_frame) * 
((double)rand() / (double)RAND_MAX));
[/quote]

I had to change that back to what it was previously so that ROM loading would 
be back to normal again without a very long delay:

[quote]
mcycles_68k = mcycles_z80 = 0;
[/quote]

As I'm still not quite familiar with the ins and outs of Genesis Plus core, 
it's unsure to me as to whether this is a bug that might be affecting the 
Wii/NGC versions too, or whether there is an architectural problem at play 
here. So I'm just mentioning it here in case you might be able to provide some 
additional explanation as to why this change in the code would be adversely 
affecting the loading of ROMs on PS3 - whereas before this revision, ROM 
loading was perfectly fine.

(For the record - changing this back to 'mcycles_68k = mcycles_z80 = 0' fixed 
the slow ROM loading for the PS3 version. But I'd still like to know what might 
be at play here)

Original issue reported on code.google.com by danieldematteis@gmail.com on 30 Dec 2010 at 2:12

[GoogleCodeExporter]

As for the uint32 != unsigned int, it seems that the GCC compiler is unable to 
recognize these as the same type in function definitions even though they are 
the same underlying type through a typedef. It is in any case unclean to use 
two different definitions. For portability reasons as well; some platforms 
define ints to 16-bit, maybe 64-bit some time in the future.

Original comment by airmake...@gmail.com on 30 Dec 2010 at 2:20

[GoogleCodeExporter]

1) uint32  is defined as unsigned int (not long) somewhere else, check better 
(types.h i think)

2) cycle counter initialization is not a bug and shouldn't have anything to do 
with ROM loading speed, you must be doing something wrong in your platform 
dependent code. The random value is there because CPUs (m68k & z80) don't start 
in sync with the VDP. Those 2 variables represent the position of both CPU 
regarding the VDP (mcycles_vdp) and are only used to run emulation, not when 
loading a new game.

PS: i looked at you porting code, and you must know that the framerate values i 
use are specific for gc/wii, they have been measured on this hardware and 
should be adapted to PS3 real framerate, if this is required...

Original comment by ekeeke31@gmail.com on 30 Dec 2010 at 4:31

• Changed state: Invalid

[GoogleCodeExporter]

Regarding function definitions in gen_input.h, you are right though, it's a 
left over when every function handlers were using uint32 at some point of 
development and i forgot to change some header files. This will be fixed.

However, i'm using gcc as well and never had any problem or warning with this. 
uint32 is defined as unsigned int in types.h and this file is included by every 
other files.

Original comment by ekeeke31@gmail.com on 30 Dec 2010 at 5:38

[GoogleCodeExporter]

Thanks for the response.

Just a few clarifying notes - I think I might have gotten to the root of the 
problem. The 'types.h' we used was from a very old revision that for some 
reason didn't get updated - 

https://code.google.com/p/genplus-gx/source/browse/trunk/source/types.h?r=184

I will check if there are still some parts in the current GX PS3 trunk that are 
similarly out-of-date, and update them accordingly. Just so you know - I had to 
apply manual diff patches to arrive from one current codebase - which was left 
at r427 - to the most recent one for two reasons -

1) Genesis Plus GX PS3 is a fork of a codebase that didn't have a public repo 
(although I could start from scratch at this point).
2) We had to isolate this slow ROM loading bug that occurred when using the 
latest trunk as the codebase for the port - that's one reason why we couldn't 
use the lates trunk straight away - as we knew it worked flawlessly at one 
point and didn't from a certain point on (r496).

I will go over the main porting code some more (GenesisPlus.cpp) and try to 
isolate what could be the problem other than this line in genesis.c. Most of 
the code is just a straight port at this point - every game bar Virtua Racing 
works at the moment - some more work is definitely required before it will have 
the same featureset as the Wii version at the moment.

Original comment by danieldematteis@gmail.com on 30 Dec 2010 at 8:32