Skip to content

Switched to GCC6 with LibNix for Amiga, disabled the timidity, mp3 and vorbis codecs#27

Closed
BSzili wants to merge 4 commits into
sezero:masterfrom
BSzili:master
Closed

Switched to GCC6 with LibNix for Amiga, disabled the timidity, mp3 and vorbis codecs#27
BSzili wants to merge 4 commits into
sezero:masterfrom
BSzili:master

Conversation

@BSzili
Copy link
Copy Markdown
Contributor

@BSzili BSzili commented Mar 16, 2022

I started working on the AmigaOS3.x port of uhexen2 again to make it playable on 68060 accelerators. As a first step I fixed the game to properly build with Bebbo's GCC6 toolchain which produces faster code:
https://github.com/bebbo/amiga-gcc
I also switched back to LibNix as the one comes with that toolchain is actively maintained and fixes many of the original bugs. The Vorbis, MP3 and Timidity codecs are disabled for now, as these are too slow on the real hardware. I plan to restore MP3 support later using MHI which uses hardware decoders on sounds cards and external decoders like the MAS Player.
Let me know if I missed something or there's something else I should change in the PR.

Comment thread common/net_sys.h
#ifndef PLATFORM_AMIGAOS3
#include <sys/param.h>
#else
#define __NO_NET_API
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is universal I guess?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EDIT: Or is it related to commenting out of -I$(OSLIBS)/amigaos/netinclude down below?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it to prevent some function prototypes from declared in arpa/inet.h that would clash with the bsdsocket.library GCC inlines. I just noticed that __NO_NET_API is already in q_endian.h but for CLIB2 only. Maybe I should add libnix to q_endian.h instead and remove this one?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't really remember why I did that in q_endian.h (was for RoadShow SDK support??), so I don't know what to do with it for now :(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can just stick to using q_endian.h for this purpose, the important thing is that __NO_NET_API is defined.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That didn't work out, as amigaos is not defined there. I think I'll leave it in net_sys.h as it's a network-related macro.

Comment thread engine/hexen2/Makefile Outdated
INCLUDES += -I$(OSLIBS)/amigaos/include
# AmiTCP SDK
NET_INC = -I$(OSLIBS)/amigaos/netinclude
#NET_INC = -I$(OSLIBS)/amigaos/netinclude
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we not using the RoadShow SDK?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original SDK had some compilation issues with some errno codes, so I opted to use the modified Roadshow SDK that comes with the GCC6 toolchain. I could probably work around this in a different way to keep the original SDK.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified the Makefiles to use the BEBBO_TOOLCHAIN switch for this as well.

Comment thread oslibs/amigaos/include/stdint.h Outdated
@@ -1,4 +1,4 @@
#ifdef __CLIB2__
#if defined(__CLIB2__) || defined(__libnix__)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old libnix doesn't have stdint.h - how to handle that, I don't know.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I first thought about using __has_include, but that is introduced in gcc5 - sigh.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new toolchain's libnix v4 has stdint.h, so I had to do this to prevent redefining the existing types. Maybe it could be done the other way? E.g. #if (defined(__has_include) && __has_include(<stdint.h>))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__has_include is supported as of gcc5, so not an option for gcc4.6

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, then don't have a good idea for a workaround for now.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stdlib.h has __MY_INLINE__, but that's obviously not reliable.

ixemul seems to have added stdint.h as of v50.0. https://github.com/bebbo/ixemul
seems to have started with v60 something, but then has gone back to v48.2 while
keeping stdint.h, sigh...

On the other hand, we are using the same header guard as the stdint.h from the
bebbo ixemul repo, i.e. __STDINT_H, so there should be no conflicts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ixemul.library is a bit of a mess, v50 and v60 were closed source forks, so many people stayed with v4x. Anyway, I'll try to figure out the root cause of the header clash.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it helps, this is the diff between stdint.h from bebbo's ixemul and ours:

--- ixemul-git/include/stdint.h
+++ uhexen2-git/oslibs/amigaos/include/stdint.h
@@ -1,12 +1,14 @@
+#ifdef __CLIB2__
+#include_next <stdint.h> /* clib2 provides a stdint.h already */
+
+#else
+
 #ifndef __STDINT_H
 #define __STDINT_H 1
 
+#include <machine/types.h> /* defines signed variants of int8_t..int64_t */
 #include <limits.h>
 
-typedef signed char int8_t;
-typedef signed short int16_t;
-typedef signed int int32_t;
-typedef signed long long int64_t;
 #define INT8_MIN SCHAR_MIN
 #define INT8_MAX SCHAR_MAX
 #define INT16_MIN SHRT_MIN
@@ -16,19 +18,19 @@
 #define INT64_MIN LLONG_MIN
 #define INT64_MAX LLONG_MAX
 
-typedef unsigned char uint8_t;
-typedef unsigned short uint16_t;
-typedef unsigned int uint32_t;
-typedef unsigned long long uint64_t;
+typedef u_int8_t uint8_t;
+typedef u_int16_t uint16_t;
+typedef u_int32_t uint32_t;
+typedef u_int64_t uint64_t;
 #define UINT8_MAX UCHAR_MAX
 #define UINT16_MAX USHRT_MAX
 #define UINT32_MAX UINT_MAX
 #define UINT64_MAX ULLONG_MAX
 
-typedef signed char int_least8_t;
-typedef signed short int_least16_t;
-typedef signed int int_least32_t;
-typedef signed long long int_least64_t;
+typedef int8_t int_least8_t;
+typedef int16_t int_least16_t;
+typedef int32_t int_least32_t;
+typedef int64_t int_least64_t;
 #define INT_LEAST8_MIN SCHAR_MIN
 #define INT_LEAST8_MAX SCHAR_MAX
 #define INT_LEAST16_MIN SHRT_MIN
@@ -38,19 +40,19 @@
 #define INT_LEAST64_MIN LLONG_MIN
 #define INT_LEAST64_MAX LLONG_MAX
 
-typedef unsigned char uint_least8_t;
-typedef unsigned short uint_least16_t;
-typedef unsigned int uint_least32_t;
-typedef unsigned long long uint_least64_t;
+typedef u_int8_t uint_least8_t;
+typedef u_int16_t uint_least16_t;
+typedef u_int32_t uint_least32_t;
+typedef u_int64_t uint_least64_t;
 #define UINT_LEAST8_MAX UCHAR_MAX
 #define UINTLEAST16_MAX USHRT_MAX
 #define UINTLEAST32_MAX UINT_MAX
 #define UINTLEAST64_MAX ULLONG_MAX
 
-typedef int8_t int_fast8_t;
-typedef int  int_fast16_t;
-typedef int int_fast32_t;
-typedef long long int_fast64_t;
+typedef int32_t int_fast8_t;
+typedef int32_t int_fast16_t;
+typedef int32_t int_fast32_t;
+typedef int64_t int_fast64_t;
 #define INT_FAST8_MIN INT_MIN
 #define INT_FAST8_MAX INT_MAX
 #define INT_FAST16_MIN INT_MIN
@@ -60,22 +62,22 @@
 #define INT_FAST64_MIN LLONG_MIN
 #define INT_FAST64_MAX LLONG_MAX
 
-typedef uint8_t uint_fast8_t;
-typedef unsigned int uint_fast16_t;
-typedef unsigned int uint_fast32_t;
-typedef unsigned long long uint_fast64_t;
+typedef u_int32_t uint_fast8_t;
+typedef u_int32_t uint_fast16_t;
+typedef u_int32_t uint_fast32_t;
+typedef u_int64_t uint_fast64_t;
 #define UINT_FAST8_MAX UINT_MAX
 #define UINT_FAST16_MAX UINT_MAX
 #define UINT_FAST32_MAX UINT_MAX
 #define UINT_FAST64_MAX ULLONG_MAX
 
 
-typedef int intptr_t;
-#define INTPTR_MIN INT_MIN
-#define INTPTR_MAX INT_MAX
+typedef long intptr_t;
+#define INTPTR_MIN LONG_MIN
+#define INTPTR_MAX LONG_MAX
 
-typedef unsigned int uintptr_t;
-#define UINTPTR_MAX UINT_MAX
+typedef unsigned long uintptr_t;
+#define UINTPTR_MAX ULONG_MAX
 
 typedef long long intmax_t;
 #define INTMAX_MIN LLONG_MIN
@@ -108,3 +110,5 @@
 #define UINTMAX_C(x) x##ULL
 
 #endif /* __STDINT_H */
+
+#endif /* __CLIB2__ */

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem appears to be sys/types.h which includes sys/_stdint.h. Fortunately that one has _..._T_DECLARED macro guards, so I can work this around properly in the oslibs stdint.h

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

Comment thread engine/hexen2/Makefile
# dynamic loading of ogl functions doesn't work
LINK_GL_LIBS=yes

USE_CODEC_TIMIDITY=no
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These need mirroring in hexenworld side too

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I'll update the hexenworld makefiles too.

Comment thread engine/hexen2/Makefile Outdated

ifndef DEBUG
CFLAGS += -O3
#CFLAGS += -O3
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-O3 is bad with gcc6?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not gcc6 specifically, but I found that O3 bloats up the code in places, so it's probably better to stick with O2 which fits in the cache better.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do -O3 only for gcc2.95 / aos3, I wonder why we did that back at the time..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was probably an attempt to make it to run faster, but with O3 you start to hit diminishing returns, and in some cases it can do more harm than good.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK then

Comment thread engine/hexen2/Makefile
CFLAGS += -fno-omit-frame-pointer
#CFLAGS += -fno-omit-frame-pointer
# these break the game with GCC6
CFLAGS += -fbbb=- -fno-tree-forwprop
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These aren't supported by old gcc2.95: We need some flag or some makefile variable to support it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add an variable e.g. GCC = 6, but I found that GCC6 usually produces better code than older versions, so I thought it's enough to keep the old flags commented out in case someone really wants to use GCC2.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some AMIGA_GCC6, or BEBBO_TOOLCHAIN, or something may be good. (I don't have bebbo's toolchain available on my setup yet, that why I am whining :))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll probably go with BEBBO_TOOLCHAIN then, since he improved a lot of things besides GCC (e.g. libnix).

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

@sezero
Copy link
Copy Markdown
Owner

sezero commented Mar 18, 2022

This is fairly good now. Attaching a tidy-up patch (on top of yours): 27a_patch.txt

(I wonder whether we need those optimizer flags out of the engine directory, but that's ultra low priority.)

@BSzili
Copy link
Copy Markdown
Contributor Author

BSzili commented Mar 18, 2022

Sure thing, I applied the patch.

sezero pushed a commit that referenced this pull request Mar 18, 2022
Tied to the new Makefile variable BEBBO_TOOLCHAIN,which defaults to yes.
Also disabled the timidity, mp3 and vorbis codecs as those are too slow
on the real hardware.

Original discussion is at #27
@sezero
Copy link
Copy Markdown
Owner

sezero commented Mar 18, 2022

Patchset squashed, commit message edited, and applied as 531d625
Thanks!

@sezero sezero closed this Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants