allegro-4.2.2-xc icon indicating copy to clipboard operation
allegro-4.2.2-xc copied to clipboard

TARGET_ARCH = $(GCC_MTUNE)=i586 does not actually produce a backwards compatible allegro.a

Open ArtificialRaccoon opened this issue 1 year ago • 6 comments

While -mtune=i586 should produce a binary compatible with i586 and earlier CPU's, this does not appear to be the case with the Allegro library. This resulted in a "gotcha" scenario for me when I moved from testing on DOSBOX to real hardware (and better VMs such as 86Box and VirtualBox), where anything built with this build of the Allegro library would immediately segfault.

Setting TARGET_ARCH=i386 in makefile.dj solves the segfault issue, and an executable built with this Allegro library work on real hardware and virtual machines other than DOSBOX.

ArtificialRaccoon avatar Oct 05 '22 14:10 ArtificialRaccoon

When not setting -march=something, GCC will default to a -march value set when building GCC itself. Conventionally, if you are invoking a GCC via i586-pc-msdosdjgpp-gcc this is probably i586 for your GCC. So this may be what is happening by default for you.

It is not actually clear from your report whether you did set -march=i386 and/or -mtune=i386 to fix the apparent problem. Setting only -mtune=i386 may maybe appear to work by accident, but GCC may still generate i586 instructions due to its internal defaults. You need to additionally set -march=i386 in order to make it explicitly avoid any non-386 instructions.

Disclaimer: I am not maintaining this particular allegro fork, but I am also using it (or rather my own fork of the fork) for openmpt123 and other projects.

manxorist avatar Oct 05 '22 14:10 manxorist

As for the actual issue, I agree that the current default at https://github.com/msikma/allegro-4.2.2-xc/blob/12ba94213768b7653eb1d667a0cbea02e04f62fe/makefile.dj#L179 should likely better be a more explicit TARGET_ARCH = -march=i386 -m80387 -mtune=pentium instead of TARGET_ARCH = $(GCC_MTUNE)=i586.

manxorist avatar Oct 05 '22 14:10 manxorist

@manxorist Thank you for the heads up about -mtune and GCC; this was very helpful to know/learn. :)

ArtificialRaccoon avatar Oct 05 '22 15:10 ArtificialRaccoon

As for the actual issue, I agree that the current default at

https://github.com/msikma/allegro-4.2.2-xc/blob/12ba94213768b7653eb1d667a0cbea02e04f62fe/makefile.dj#L179 should likely better be a more explicit TARGET_ARCH = -march=i386 -m80387 -mtune=pentium instead of TARGET_ARCH = $(GCC_MTUNE)=i586.

Do you think it would be a good idea to change this?

To be completely honest, I don't actually have that much experience with C compiler settings. I did the minimum needed to get it cross compiling on my setup. But if this is a more useful default it might be nice to change it.

msikma avatar Oct 19 '22 11:10 msikma

As for the actual issue, I agree that the current default at https://github.com/msikma/allegro-4.2.2-xc/blob/12ba94213768b7653eb1d667a0cbea02e04f62fe/makefile.dj#L179

should likely better be a more explicit TARGET_ARCH = -march=i386 -m80387 -mtune=pentium instead of TARGET_ARCH = $(GCC_MTUNE)=i586.

Do you think it would be a good idea to change this?

Yes. Given that that aspect of the Makefile was added over 22 years ago, it was probably assumed that -march=i386 would be the default, which does not hold any more for all compiler builds, It should be TARGET_ARCH = -march=i386 -m80387 $(GCC_MTUNE)=pentium to still be compatible with -mcpu= instead of mtune= for very old GCC. I do not know when exactly -m80387 was added in GCC though. It's documented for GCC 7, but GCC does not seem to error with it even down to 4.1 (I have not tested any earlier).

To be completely honest, I don't actually have that much experience with C compiler settings. I did the minimum needed to get it cross compiling on my setup.

Yeah, uhm, there are some people keeping allegro 4.2 forks around, each with some the same and some other set of small patches, I do so also myself, but this is not public on github at the moment. It maybe would make some sense to consolidate these efforts of various people in some way or another.

manxorist avatar Oct 19 '22 12:10 manxorist

Thanks! I'm currently traveling but when I'm back I'll look into this.

Although initially I just put this repo up for my own use, given that other people have found it and commented on it, I'm happy to make some more adjustments to make it more useful. To me this was always an experiment and I'm not very well versed in C let alone the intricacies of how to properly set up a compiler for an older system. So I appreciate the help.

msikma avatar Nov 02 '22 00:11 msikma