Skip to content

Linux: Drop ppc32 (32-bit PowerPC) architecture support #106390

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented May 14, 2025

This was added together with ppc64le in #54490, but seemingly only for the purpose of getting it to compile on a Linux distro that aims at maximizing support for all CPU architectures.

I don't think anyone has ever run Godot on a ppc32 system (do those even support OpenGL ES 3.0?) and so I don't think we should aim to support it.

Debian dropped support for its PowerPC (ppc32) arch in Debian 9, released in 2017.

CC @q66 in case you feel strongly about this. The maintenance burden for us isn't too high (as seen in this PR), but I'm not too comfortable having a platform that we can theoretically compile to but with likely no actual use case for it.

If someone can actually test Godot on PowerPC and see if there's any valid use case (e.g. running a headless server or somehow working 2D with software rendering), I'm happy to keep the theoretical support as a "tier 2" platform, like the *BSD flavors we support on a best-effort basis.

This was added together with `ppc64le` in godotengine#54490, but seemingly only for the
purpose of getting it to compile on a Linux distro that aims at maximizing
support for all CPU architectures.

I don't think anyone has ever _run_ Godot on a `ppc32` system (do those even
support OpenGL ES 3.0?) and so I don't think we should aim to support it.

Debian dropped support for its PowerPC (`ppc32`) arch in Debian 9, released
in 2017.
Comment on lines -240 to -242
#else
return "riscv";
#endif
Copy link
Member Author

Choose a reason for hiding this comment

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

CC @aaronfranke

I removed this because we don't support any riscv architecture name, so this was incorrect.

BTW https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/E8EO-Fd4t3s/m/AppiqCh_BQAJ hints that __riscv_xlen shouldn't be used like this. I kept it in OS::has_feature as the logic is a bit different there, we explicitly support both fully-specified and generic feature names (rv64 and riscv, wasm32 and wasm, etc.).

Copy link
Member

Choose a reason for hiding this comment

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

It was meant as a fallback in case somebody tries to build on 32-bit or 128-bit RISC-V so it doesn't claim to be rv64, but it still wouldn't be fully correct in that case indeed.

sizeof(void*) seems worse than checking a define the compiler provides, and __riscv_xlen works. From the Google groups discussion it looks like it's meant to indicate the ISA, but for RISC-V the ISA is tied to the pointer size, so I'm not sure when the distinction matters. If we did need to migrate and support multiple bit widths, it would make sense for Godot to provide some kind of define so we could match other architectures that use a define.

Btw: Would love to port Godot to 128-bit one day just because that sounds like a cool project, but uh, need to wait for Linux to support it first (so far the only 128-bit content that exists is some low level assembly stuff in VMs).

Comment on lines +239 to 240
#elif defined(__wasm64__)
return "wasm64";
Copy link
Member Author

Choose a reason for hiding this comment

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

This is technically unused currently so it was future proofing, but #105670 will make it useful.

CC @adamscott

@@ -81,7 +81,7 @@ _ALWAYS_INLINE_ static void _cpu_pause() {
__builtin_ia32_pause();
#elif defined(__arm__) || defined(__aarch64__) // ARM.
asm volatile("yield");
#elif defined(__powerpc__) || defined(__ppc__) || defined(__PPC__) // PowerPC.
#elif defined(__powerpc__) // PowerPC.
Copy link
Member Author

Choose a reason for hiding this comment

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

I just simplified because __powerpc__ should be sufficient here, __ppc__ and __PPC__ are legacy aliases for old hardware we can in no way aim to support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants