Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCC Compiler Support #1791

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

GCC Compiler Support #1791

wants to merge 2 commits into from

Conversation

Thar0
Copy link
Contributor

@Thar0 Thar0 commented Jan 21, 2025

This PR adds support for building the project with a modern MIPS GCC compiler.

Thanks to @fig02 for testing a build on hardware, and everyone who engaged in related discussions.

Copy link
Collaborator

@hensldm hensldm left a comment

Choose a reason for hiding this comment

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

Thoughts on bringing mkldscript.c over from OoT (though does require adding the else if for STMT_increment on lines 40-41) to have parity between the two (mainly looks like just formatting is the difference between the 2). If you don't want to that is fine, I would probably do it after this gets merged.

@hensldm hensldm removed the Needs-first-approval First approval label Jan 21, 2025
ENDIAN := -EB
ifeq ($(COMPILER),gcc)
# MIPS options
CFLAGS += -G 0 -march=vr4300 -mtune=vr4300 -mfix4300 -mabi=32 -mno-abicalls -mdivide-breaks
Copy link
Collaborator

Choose a reason for hiding this comment

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

This reminds me, why do we use -mdivide-breaks? The n64 supports trap instructions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's mostly for compatibility. I think it's a reasonable default that people can change themselves depending on their tolerance for what platforms they want to support

COMMON_DEFINES := $(GBI_DEFINES)
AS_DEFINES := $(COMMON_DEFINES) -DMIPSEB -D_LANGUAGE_ASSEMBLY -D_ULTRA64
C_DEFINES := $(COMMON_DEFINES) -D_LANGUAGE_C
ENDIAN :=
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why omit this?

Suggested change
ENDIAN :=
ENDIAN := -EB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an architecture default, it doesn't really need to be explicit. We already rely on -mhard-float being implicit from the arch

@AngheloAlf AngheloAlf added Merge-ready All reviewers satisfied, just waiting for CI and removed Needs-second-approval Second approval labels Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merge-ready All reviewers satisfied, just waiting for CI Tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants