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

Change known gte asm files to asm #2160

Merged
merged 2 commits into from
Feb 9, 2025
Merged

Change known gte asm files to asm #2160

merged 2 commits into from
Feb 9, 2025

Conversation

sozud
Copy link
Collaborator

@sozud sozud commented Feb 1, 2025

We know all of these are handwritten asm from dumping the libs. Script + results here: https://gist.github.com/sozud/6f5d111c49b69fd7b1c0c812067b2eeb

Change these to asm so people don't waste time trying to match them. The changes to src/main/psxsdk/libgte/reg03.c may be controversial since someone found a C match for it, but it's not going to match the original object. @JoshSchreuder Seems to have done most of the work on that file

@JoshSchreuder
Copy link
Contributor

No controversy here, I think this was the very first thing I worked on as it was at the top of the functions list and I thought it "easiest". Before I understood this is not technically SOTN code. Change makes sense to me

@sozud
Copy link
Collaborator Author

sozud commented Feb 2, 2025

I remember @ser-pounce was looking at InitGeom, should get approval from them too

@ser-pounce
Copy link
Contributor

I think any direction is fine for these as long as it's consistent. As the raw assembly can be somewhat simplified, should they be marked as hasm in the splat file instead of asm? It would also indicate that this is intentional and that they aren't waiting to be converted to C.

@sozud
Copy link
Collaborator Author

sozud commented Feb 2, 2025

It seems like hasm doesn't include the preamble stuff?

https://github.com/ethteck/splat/blob/8ccc84dd15628ca3c7b948d41dac541fe108b986/src/splat/segtypes/common/asm.py#L31

/* Generated by spimdisasm 1.30.2 */

/* Handwritten function */
glabel InitGeom
    /* 766C 80016E6C 0380013C */  lui        $at, %hi(D_8002DBD0)
    /* 7670 80016E70 D0DB3FAC */  sw         $ra, %lo(D_8002DBD0)($at)
    /* 7674 80016E74 8B64000C */  jal        patch_gte
    /* 7678 80016E78 00000000 */   nop
asm/us/main/psxsdk/libgte/msc00.s: Assembler messages:
asm/us/main/psxsdk/libgte/msc00.s:4: Error: unrecognized opcode `glabel InitGeom'
asm/us/main/psxsdk/libgte/msc00.s:5: Warning: used $at without ".set noat"
asm/us/main/psxsdk/libgte/msc00.s:6: Warning: used $at without ".set noat"
asm/us/main/psxsdk/libgte/msc00.s: Error: .size expression for InitGeom does not evaluate to a constant
make: *** [Makefile.psx.mk:86: build/us/asm/us/main/psxsdk/libgte/msc00.s.o] Error 1

@ser-pounce
Copy link
Contributor

Seems like that's the current version of splat, because if we were using it it should also generate the preamble for .hasm. This commit seems to have been the one that changed how splat handled the generation so presumably anything earlier will fail as in your log

ethteck/splat@0660458

It could be worked around by adding --macro-inc to the maspsx options I guess.

@sozud
Copy link
Collaborator Author

sozud commented Feb 2, 2025

There's some sort of splat bug or something with 26.1 and greater

  File ".local/lib/python3.10/site-packages/splat/segtypes/common/data.py", line 24, in out_path
    if self.type.startswith("."):
RecursionError: maximum recursion depth exceeded while calling a Python object

I suppose what could be done is including the asm as actual files in the repo and adding the appropriate prelude

@sozud sozud merged commit ad0d270 into Xeeynamo:master Feb 9, 2025
26 checks passed
ProjectOblivion pushed a commit to ProjectOblivion/sotn-decomp that referenced this pull request Feb 16, 2025
We know all of these are handwritten asm from dumping the libs. Script +
results here:
https://gist.github.com/sozud/6f5d111c49b69fd7b1c0c812067b2eeb

Change these to asm so people don't waste time trying to match them. The
changes to src/main/psxsdk/libgte/reg03.c may be controversial since
someone found a C match for it, but it's not going to match the original
object. @JoshSchreuder Seems to have done most of the work on that file
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.

4 participants