Repository navigation
Upgrade GCC 13.2.0 version (PPU-SPU) - #121
humbertodias wants to merge 164 commits into
Conversation
|
Hey cool work, I really want gcc 13 on ps3. But you just forgot to credit Darjan Krijanand and luizfernandonb :/ |
Yeah.. I left his name on patch's file given him the credit |
This reverts commit ec5410b.
|
Could the docker ci be skipped for pull requests? |
…h # fails closed in a few seconds when those secrets are unavailable
Sure! Skipped 6f580ea |
|
So, looking at the current "Files changed" for this PR, there are a few unrelated changes here:
These should be made into separate PRs so that they can be reviewed and merged (if appropriate). In addition there's a bunch of other stuff which I don't think is relevant, like the utils functions to handle Kitchen sink PRs like this one can be useful to try get some testing of a complete set of changes, but they are not conductive to actual merging. 😸 |
GDB was causing issues building on macOS from what I remember and users can install GDB-Multiarch to get the same result, we also have no way to actually use GDB outside RPCS3 from what I know. I can look into restoring it. |
Sounds like a better solution would be to fix the build issues then. Probably there is some upstreams commit that can be backported. GDB should be able to interact with |
|
@magendavid06-cell I do not believe that this should be merged without significant cleanup. This PR has 160 commits with changes to 26 files, none of which are needed to actually implement what the PR says it is for (because that has already gone in though PRs in other repos). Merging this will put the main repo in a very confusing state. I'm not (at the moment at least 😸) objecting to any specific change still contained in this PR, I'm just saying that the different changes should be squashed and re-published as separate PR:s so that they can be properly reviewed and cleanly merged. (sorry, had the wrong Github user active at first...) |
|
@zeldin This person created their account a few hours ago just to approve the changes with no comments. I didn't even know someone could do that until now. I would probably disregard it entirely. That said, this probably is a reminder this is an important item to raise and get some motion on. I agree with the three separated PRs you mentioned earlier - am I right in saying the next step is we make those PRs, strip out the extraneous features from this one, and then return to this one for a review of GCC? It seems like the core work is done, it just needs procedure. |
|
@TheMrIron2 The GCC stuff is already done; it was reviewed and merged as ps3dev/gcc-PS3#1. If there is any fallout from it we can handle it with new issues/PRs, I don't think there is any benefit in remitting to this PR. If any input from humbertodias is needed we can tag him. As for the other changes, yes, I'd say the next step is to make new PR:s. It's a little bit of extra work but it will make the scope of each change clear so that it can be reviewed, tested and merged in isolation. I don't think there are any cross-dependencies. If humberodias doesn't have the time or energy to do it, it's possible for someone else to pick up the torch; we can still give proper credit in the author field. |
CI, the Docker image, the PPU binutils 2.42 bump, the gdb restore, and the config.guess helpers do not belong here. The host GCC 16 and Apple Silicon fixes, resumable downloads, and the ps3libraries cross compiler stay.
Hi, sorry for the delay! No problem, I’ve removed the unrelated changes in commit dc48523 |
|
Thanks. Now we have something manageable to review. 😄 Will you make additional PR:s for the binutils bump and/or CI/docker changes? Let's consider the three parts of what is left in this PR:
|
| } | ||
|
|
||
| file_size() { | ||
| wc -c < "$1" | tr -d ' ' |
There was a problem hiding this comment.
Using wc -c with a redirection from stdin forces wc to read the whole file to count the bytes. Better to use
wc -c "$1" | awk '{print $1}'
so that it can use use stat().
There was a problem hiding this comment.
good idea. changed to use stat()
| done < "$ARCHIVE" | ||
| } | ||
|
|
||
| # Guess a GNU/sourceware URL when archives.txt has no matching line. |
There was a problem hiding this comment.
Why? If a line is missing in archives.txt, shouldn't we just add it so that we get the SHA as well?
There was a problem hiding this comment.
makes sense. i removed the guess_url() function
Those fixes belong in the gcc repo's generated patch, not in split files applied by the build scripts. The download, SPU binutils, and toolchain.sh changes move to their own branches.
|
|
Awsome! Thanks. |
Oh I see, my bad! Should I wait for humber to split his GCC Restoration or should I work on it? |
I actually started working on restoring GDB myself today. 😄 The macOS issue turned out to be with zlib, so I'm attempting to just add |
|
I see nice! Again apologies for doing that, never pushing straight to main again, that's for sure. |
|
already merged |
|
thanks @humbertodias for all the work across the gcc PRs, and @zeldin for restoring GDB 🙏 |
Features
Note
We've selected the version 9.5.0 because it's the last GCC release to include support for SPU.
https://www.phoronix.com/news/GCC-10-Drops-Cell-BE-SPU
Tip
Patches credits to disc-kuraudo and luizfernandonb
Result
https://github.com/humbertodias/ps3toolchain/actions/runs/8166663270
Note
Don't forget to create a named DockerHub environment on your repository with two secret variables: DOCKERHUB_USERNAME and DOCKERHUB_TOKEN
badges

PSL1GHT hello game