ZSNES on Intel Mac, further progress

[Nach]

I've now sent a testcase to Apple and the fix should eventually appear in CVS.

Keep me updated on the status of this.

will you also consider committing the change to the LDFLAGS (-read_only_relocs suppress)

This flag sounds to me like Apple knows they have a bug with their linker / or objects generated by some of their compilers, and added a flag to work around it.
If I'm mistaken, please tell me so. Using this flag creates working binaries?

and adding and using the function sdl_keysym_to_pc_scancode().

It's on my todo list, it's just a lot of code, and I didn't get a chance to thoroughly review it yet. I'd like to do some more research into the issue first and see if a similar function already exists in SDL.

[hector]

The only problem with read only relocs is that they disallow sharing of the text segment among multiple invocations of the program. There isn't anything wrong with them really.

[Nach]

Is there any particular reason NASM is putting those in read only relocs?

[hector]

The assembly code calls the routines getchar and putchar. These are external relocatable symbols that occur within the text section because assembler code goes into the text section. They are fixed up at runtime, which necessitates modification of the read-only text section when it is loaded at runtime.

So really it is something that happens in Linux all the time.

[Nach]

The assembly code calls the routines getchar and putchar. These are external relocatable symbols that occur within the text section because assembler code goes into the text section. They are fixed up at runtime, which necessitates modification of the read-only text section when it is loaded at runtime.

So really it is something that happens in Linux all the time.

I haven't really read up on the ELF format that much, so I'm somewhat in the dark on this. I just looked at uic.o a bit, and seems the externs do appear in read only text section.

So why does Apple complain about it?

And what's the verdict with this?

Code: Select all

+; Ensure data section precedes bss if any
+section .data 

Is this needed? Is this an Apple bug?

[hector]

From the ld manpage it says that read only relocs are undesirable as they prevent sharing of the text segment, thereby increasing memory usage if more than one invocation of the program is running at the same time. So they error it to alert the developer, but at the same time allow the developer to acknowledge this and turn off the error in the case that they know what they are doing. And doing a google search for this linker flag shows other projects making use of it as well.

I think that the little bit I added at the end of macros.mac is a workaround around a bug in the outmacho module, and I haven't started looking for the bug yet. I'm not really pushing that change for the reason you stated initially - that we should not workaround assembler bugs but fix them instead.

[Nach]

I think that the little bit I added at the end of macros.mac is a workaround around a bug in the outmacho module, and I haven't started looking for the bug yet. I'm not really pushing that change for the reason you stated initially - that we should not workaround assembler bugs but fix them instead.

Well, that's our stance I believe, unless we're forced otherwise.
For the same reason, we're not redoing our OGL code because of broken ATI drivers.

If you get a chance, I would appreciate if you look for more NASM bugs, since you seem to know what you're doing, and I don't have that much faith at the moment in Apple or NASM team fixing it in a timely manner without help.
You'd think when testing a compiler/assembler, they'd try building huge files and checking to make sure they were built right...

[pagefault]

I think that the little bit I added at the end of macros.mac is a workaround around a bug in the outmacho module, and I haven't started looking for the bug yet. I'm not really pushing that change for the reason you stated initially - that we should not workaround assembler bugs but fix them instead.

Well, that's our stance I believe, unless we're forced otherwise.
For the same reason, we're not redoing our OGL code because of broken ATI drivers.

If you get a chance, I would appreciate if you look for more NASM bugs, since you seem to know what you're doing, and I don't have that much faith at the moment in Apple or NASM team fixing it in a timely manner without help.
You'd think when testing a compiler/assembler, they'd try building huge files and checking to make sure they were built right...

Our OGL code is broken though. It just happens to run even worse on ATI drivers.

[Nach]

I think that the little bit I added at the end of macros.mac is a workaround around a bug in the outmacho module, and I haven't started looking for the bug yet. I'm not really pushing that change for the reason you stated initially - that we should not workaround assembler bugs but fix them instead.

Well, that's our stance I believe, unless we're forced otherwise.
For the same reason, we're not redoing our OGL code because of broken ATI drivers.

If you get a chance, I would appreciate if you look for more NASM bugs, since you seem to know what you're doing, and I don't have that much faith at the moment in Apple or NASM team fixing it in a timely manner without help.
You'd think when testing a compiler/assembler, they'd try building huge files and checking to make sure they were built right...

Our OGL code is broken though. It just happens to run even worse on ATI drivers.

Oh I know it's bad. But we're not going to do all sorts of crazy stuff in it just to work around ATI issues. And that was your decision.

[hector]

The assembler bug that necessitated the following patch:

Code: Select all

Index: macros.mac
===================================================================
--- macros.mac	(revision 3183)
+++ macros.mac	(working copy)
@@ -63,3 +63,6 @@
 %macro ALIGN16 0
   times ($$-$) & 1Fh nop    ; Long word alignment
 %endmacro
+
+; Ensure data section precedes bss if any
+section .data

has been found!

Code: Select all

--- outmacho.bak        2006-06-16 21:21:48.000000000 +0100
+++ outmacho.c  2006-06-16 21:22:08.000000000 +0100
@@ -981,7 +981,6 @@ static void macho_write_section (void)
                /* add sizes of previous sections to current offset */
                for (s2 = sects, fi = 1;
                     s2 != NULL && fi < r->snum; s2 = s2->next, fi++)
-                   if ((s2->flags & SECTION_TYPE) != S_ZEROFILL)
                        l += s2->size;
            }
 

Basically, if a bss section precedes a data section, then when references are made to the data section, they didn't skip over the bss section (because of that erroneous if statement above) and so they referenced something completely different. This patch teaches outmacho to take account of the bss section's size too when calculating the final addresses of the relocations.

Now there seems to be only one visible bug left in the assembler, which causes a Code Alignment warning for me on zsnes startup. The warning seems harmless though as most things in zsnes still work. I'll take a look anyway.

[Cosmo]

This is going to be extremely handy when I get a Mac.

[hector]

The Code Alignment warning was caused by the .text section having an alignment of 1. Apparently from looking at the assembler code this is the default for Mach-O, whereas for ELF the default alignments are:
.text 16
.data 4
.bss 4
This is also the case for win32. Coff doesn't support section alignments.

So, in order to make Mach-O conform to these specs as well we must explicitly spell out the alignment when we declare the sections. The following patch ought to do it:

Code: Select all

Index: macros.mac
===================================================================
--- macros.mac  (revision 3257)
+++ macros.mac  (working copy)
@@ -19,7 +19,14 @@
 
 
 bits 32
-section .text
+
+;COFF does not support the align extension to the section directive
+%ifndef __MSDOS__
+section .text align=16
+section .data align=4
+section .bss  align=4
+%endif
+
 ; Zsnes required macros
 
 %define ZVERSION 'Pre 1.43'

Since coff doesn't support section alignment it must be left out for DOS platforms.

Only problem is that outmacho has a bug that prevents you from setting the alignment of the text section. So, here's the fix for the assembler:

Code: Select all

--- outmacho.bak        2006-06-16 23:53:56.000000000 +0100
+++ outmacho.c  2006-06-16 23:11:08.000000000 +0100
@@ -539,7 +539,7 @@ static long macho_section(char *name, in
                 s->data = saa_init(1L);
                 s->index = seg_alloc();
                 s->relocs = NULL;
-                s->align = DEFAULT_SECTION_ALIGNMENT;
+                s->align = -1;
 
                 xstrncpy(s->segname, sm->segname);
                 xstrncpy(s->sectname, sm->sectname);
@@ -579,7 +579,8 @@ static long macho_section(char *name, in
                         }
 
                         if ((-1 != originalIndex)
-                            && (s->align != newAlignment)) {
+                            && (s->align != newAlignment)
+                            && (s->align != -1)) {
                             error(ERR_PANIC,
                                   "section \"%s\" has already been specified "
                                       "with alignment %d, conflicts with new "
@@ -889,6 +890,8 @@ static unsigned long macho_write_segment
            fwritelong(offset, machofp);
            /* Write out section alignment, as a power of two.
               e.g. 32-bit word alignment would be 2 (2^^2 = 4).  */
+           if (s->align == -1)
+               s->align = DEFAULT_SECTION_ALIGNMENT;
            fwritelong(s->align, machofp);
            /* To be compatible with cctools as we emit
               a zero reloff if we have no relocations.  */

Here's a list of issues remaining with ZSNES on Mac OS X:
1. Full screen 640x480 and 800x600 don't work.
2. 8-point sound interpolation doesn't work.
3. Filters (except standard interpolation) don't function properly in full-screen modes.
Let me know if there are any more.

[Deathlike2]

There's a fair chance the High Quality Low Pass Option doesn't work either...

All of the filters you have mentioned require MMX (which have been available in Intel processors since 1996).. there's a possible chance that the NASM compiler for the Mac is probably retarded when dealing with MMX instructions.. you might want to look into it.

[BRPXQZME]

I was out for a few days working my butt off so I can graduate (whew!), so I haven't been around to clarify some things, but hey.

Just a note regarding the linker flag -read_only_relocs:

With this set to suppress, the wrappers for getchar, putchar and also malloc can be removed.

However, this flag can also hide errors that crash zsnes. If the following change is reverted

Code: Select all

Index: uic.c
===================================================================
--- uic.c       (revision 3234)
+++ uic.c       (working copy)
@@ -133,7 +133,7 @@
 }
 
 void *malloc_ptr;
-unsigned int malloc_size;
+unsigned int malloc_size = 0;
 
 void malloc_help()
 {

malloc_size appears as a common symbol in uic.o rather than a data symbol. During linking, an error is normally produced that _malloc_size is a read-only relocatable symbol. This actually seems to happen because the linker can't find _malloc_size in uic.o for some strange reason. If this error is suppressed the resultant zsnes will crash with a bus error. BRPXQZME, I believe the bus error you experienced was for this very reason. In your case the fix would be the above patch.

I added that back in when I realized it had fallen out of my patchset, but yeah... since it's renamed in SVN to not be a special name, it's fixed. At least now I know what that means....

Thank you Nach for the commits. I don't want to seem ungrateful, but will you also consider committing the change to the LDFLAGS (-read_only_relocs suppress), and adding and using the function sdl_keysym_to_pc_scancode().

I question the wisdom of having that flag in there by default, or at least keep it at warning, since this does reveal things like the malloc_size function name conflict, at least (not that it's likely to happen again).

will you also consider committing the change to the LDFLAGS (-read_only_relocs suppress)

This flag sounds to me like Apple knows they have a bug with their linker / or objects generated by some of their compilers, and added a flag to work around it.
If I'm mistaken, please tell me so. Using this flag creates working binaries?

It's not really a bug, just an undesirable. In fact, Gentoo Linux users see a similar complaint whenever they compile something with one of these, some confusing technogarble about TEXTREL in a QA warning or something like that (and especially when you use a binary-only driver). It makes things go a little slower, but on the other hand it's just one freaking function call. Computers like to not have to find things to use them, so by making a wrapper function you sacrifice efficiency for certainty (by not making it have to fill in the address). It has made working binaries for me, it's just a cheap trick I used to force the compiler into continuing past something it wants you to stop at. But I still think it should be at "warning" instead of "suppress", just so nothing else slips under the radar.

All of the filters you have mentioned require MMX (which have been available in Intel processors since 1996).. there's a possible chance that the NASM compiler for the Mac is probably retarded when dealing with MMX instructions.. you might want to look into it.

Pretty much anything I've tried to compile for Intel OS X gets annoyed at the MMX code more than anything else. Wonder why, but it's the truth. But I don't know how well NASM handles it; all those other things were choking with GCC inline assembly.

EDIT: forgot to mention. Hector, awesome stuff.

EDIT2: So I did a recompile with all the latest info except Hector's last post, and now I get the code alignment error for the first time ever... hm. Doesn't stop the show, so I guess we wait for nasm to be fixed if we're too lazy to patch it ourselves.... I will have the patch up soon once (if?) the school's connection lets me in.

EDIT3: Uh ohs. Attempts to run r3259 with the current patches results in not just a Code Alignment Failure, but also pretty much graphics being either black or messed up in most games (well, Star Fox actually causes a Bus Error). Since I did just ram through a lot of revisions, I'm not too sure everything went smoothly.

[hector]

I don't have any problems with the latest revision of the code. Make sure you apply all the patches I posted to the assembler.

All the lowpass filters work, even the high quality one, so I'm not sure what to think. The thing is, the 8-point interpolation sort of works, except that if there is no input, the sound gets stuck in a loop, and only when you press keys does the music continue.

[Nach]

hector: Nice going
Are all these patches on the way to someone at Apple or NASM?

BTW, can someone test dual mice support for me on Mac?

Get T2 or Arkanoid, or any other game listed in the dual mice thread. Plug in to mice, start ZSNES, start up in menu, see if each mouse controls it's own individual player.

[hector]

The patches have gone to Eric Christopher who is responsible for maintaining NASM over at Apple. Last he spoke he said he would look into it soon. No knowing exactly when though, knowing Apple.

I'd like to point out a small mistake in r3260. In configure.in NFLAGS was changed to

Code: Select all

NFLAGS="$NFLAGS -D__MACOSX__ -f macho -DELF MACHO"

If I'm not mistaken it should be the following instead:

Code: Select all

NFLAGS="$NFLAGS -D__MACOSX__ -f macho -DMACHO"

I've managed to compile OpenGL support into zsnes, but the code required doesn't seem to fit in nicely into configure.in. I'm looking at Xcode as an alternative but I'm currently stuck on how to compile and run parsegen first to produce the .h files.

[Nach]

The patches have gone to Eric Christopher who is responsible for maintaining NASM over at Apple. Last he spoke he said he would look into it soon. No knowing exactly when though, knowing Apple.

Okay great.

I'd like to point out a small mistake in r3260. In configure.in NFLAGS was changed to

Code: Select all

NFLAGS="$NFLAGS -D__MACOSX__ -f macho -DELF MACHO"

If I'm not mistaken it should be the following instead:

Code: Select all

NFLAGS="$NFLAGS -D__MACOSX__ -f macho -DMACHO"

I've managed to compile OpenGL support into zsnes, but the code required

Thanks for catching, I should've double checked it before commiting

[DAEGU]

Okay. It's nice that there is talk about development for macs, but it looks like it's all going to be x86... Not every one has an intel mac though. In fact, most mac users don't. Will there be at least a port of zsnes compatible with the G5's?

[kevman]

No.
Not now.
Not ever, either, I am 95% sure.

Zsnes is over 75% non-portable code.

[tehnick]

Yeah... the only reason ZSNES is coming to Mac is because they're using Intel processors now. Sooner or later, most people will have x86 macs anyway.

[hector]

I'm wondering whether there has been some mistake in the following:

Code: Select all

Index: linux/sw_draw.c
===================================================================
--- linux/sw_draw.c     (revision 3271)
+++ linux/sw_draw.c     (working copy)
@@ -45,7 +45,7 @@
 {
     //unsigned int color32, p;
     //int i;
-    Uint32 flags = SDL_DOUBLEBUF | SDL_SWSURFACE;
+    Uint32 flags = SDL_SWSURFACE;
     DWORD GBitMask;
 
     flags |= (FullScreen ? SDL_FULLSCREEN : 0);

With the SDL_DOUBLEBUF added 640x480 fullscreen doesn't work on Mac.
Over at http://www.libsdl.org/cgi/docwiki.cgi/S ... tVideoMode , it says that SDL_DOUBLEBUF is only valid with SDL_HWSURFACE, but here we see it together with SDL_SWSURFACE.
My question is, can we get rid of SDL_DOUBLEBUF from the above?

[hector]

Can some people test out the Xcode project available from:

http://hectorchu.googlepages.com

All relevant instructions are there.

[tehnick]

I'm on it.

BTW, think it might be a good idea to add 1440x900 and 1680x1080 in the list of resolutions for the OS X version? I mean, that's what a lot of people are going to be using since those are the native iMac/Apple LCD resolutions.

[Deathlike2]

Have you even looked into the custom res support that's already there?