-
Notifications
You must be signed in to change notification settings - Fork 46
fix critical bug in GTE macros, refactor test functions, add one more func #233
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
Conversation
|
|
||
| #include <ctr/math.h> | ||
| #include <psn00bsdk/include/inline_c.h> | ||
| #include <ctr/nugget/inline_n.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you more sure that this doesn't have bugs in it like psn00bsdk? If it does have bugs, will we have to manually vet gte instructions ourselves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one doesn't, yea
| TEST_COLL_ProjectPointToEdge(v1, v2, point, out); | ||
| /* This is a hand written assembly function that breaks the ABI, | ||
| and some callers expect the argument registers to be untouched */ | ||
| __asm__ volatile("move $a0, %0" : : "r"((u32)out)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you ever fully solve this problem? Was it ultimately because of psn00bsdk?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, t1 being trashed during the gte registers was causing this bug
rewrite/src/tests/test.c
Outdated
| __asm__ volatile("move $k1, %0" : : "r"(addr)); | ||
|
|
||
| u32 index = 0; | ||
| u32 index = UINT32_MAX; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to match our integral types paradigm, should this be named U32_MAX instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a stdint macro, i guess i could define our macros to match yea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, if that's the case then maybe this isn't a big deal to just keep them. Up to you.
| TEST_FUNC(COLL_TestLeaf_Quadblock), | ||
| }; | ||
|
|
||
| const char* s_nameTestedFunc = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good improvement.
| u8 lodShift; | ||
| } CollDCache; | ||
|
|
||
| #define DCACHE_COLL (*(CollDCache*) 0x1f800108) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no memory overlap behavior for the scratchpad within a namespace (i.e., we had unions in the OG scratchpad because several namespaces used it), but I thought even within a single namespace (e.g., coll), that there was still memory usage overlap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't seen any overlap yet
No description provided.