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

AK+LibWeb: First steps for PowerPC CPU architecture support #24684

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sideeffect42
Copy link
Contributor

This PR adds AK_IS_ARCH defines for ppc/ppc64/ppc64le and user agent CPU strings for 32-bit and 64-bit PowerPC.

For the 64-bit PowerPC user agent I followed the RISC-V 64 pretty-printed example instead of using ppc64(le) like x86_64 or AArch64.


This is a copy of LadybirdBrowser/ladybird#391 and has not been tested in SerenityOS, because I don't have a system running SerenityOS.

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Jul 10, 2024
@nico
Copy link
Contributor

nico commented Jul 10, 2024

This is a copy of LadybirdBrowser/ladybird#391 and has not been tested in SerenityOS, because I don't have a system running SerenityOS.

It sounds like there are 0 users of this. Why send a PR?

@sideeffect42
Copy link
Contributor Author

It sounds like there are 0 users of this. Why send a PR?

Absolutely. Of course there are 0 users when you cannot compile it :-)
First you need to fix the software, then there can be users.

I'm sending these PRs because I already did the work for Ladybird and it shares most (all?) of the AK data structures with SerenityOS.
So I thought these changes may be of interest to SerenityOS, too. I was just trying to be nice and I'm sorry if it is not well-received.

@nico
Copy link
Contributor

nico commented Jul 12, 2024

Oh, apologies if that sounded unwelcoming, that's not how I intended it at all.

It's absolutely well-received if you want to work on PPC64 bring-up. But if you don't, I think it might make sense to not add this now and wait until someone actually wants to either build lagom tools on PPC (which will need this and probably some userspace changes), or port the kernel (which will need lots of kernel work). Else it's kind of dead code, and we try not to add dead code.

If you're interested in doing either of these two things, I'll gladly merge this!

@supercomputer7
Copy link
Member

supercomputer7 commented Jul 13, 2024

I hate to be a gatekeeper, so let me explain my thoughts on this...

I agree with Nico here. If we are not going to port the kernel to powerpc (or ppc64), just making the browser (or LibWeb, for that matter) compilable on that CPU architecture simply doesn't help anything because it will simply not be used.
On the Ladybird project, it's a whole different story, because there might be actual people that want to run Ladybird on their Linux machines (which are PowerPC-based ones). But for us, we simply don't care, because we don't develop a browser for Linux (or a cross-platform, to be more accurate) here anymore.

Even if you just want Lagom to run on ppc64, I'd say this has no justification as well, because the way I see it, Lagom is mainly used for bootstrapping our development environment and most people wouldn't actually care using it alone on their Linux environment.

And presumably powerpc (and maybe ppc64) are somewhat dead CPU architectures, then I don't really see a point investing in it.

However, if you have hardware that you actually care about and want SerenityOS to run on, then starting this could make sense. In such case, gatekeeping doesn't make sense but we must have a known path on what you expect to do with this further :)

Copy link

stale bot commented Aug 5, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

@stale stale bot added the stale label Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 pr-needs-review PR needs review from a maintainer or community member stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants