ps3dev / PSL1GHT

A lightweight PS3 SDK
www.psl1ght.com
MIT License
222 stars 62 forks source link

internal narrowing conversion error in ppu/include/vectormath/cpp/quat_aos.h #70

Closed ParzivalWolfram closed 4 years ago

ParzivalWolfram commented 4 years ago

Potential issue with PSL1GHT:

Reproduction code: make -f Makefile.ps3 from https://github.com/emukidid/wii64-ps3 current master (commit 85b23eeaf0c3ef224cdde6074bcda99339b6274f for future reference)

Error:


/usr/local/ps3dev/ppu/include/vectormath/cpp/quat_aos.h:502:100: error: narrowing conversion of '2147483648' from 'unsigned int' to 'int' inside { } [-Wnarrowing]
     return Quat( vec_xor( quat.get128(), ((vec_float4)(vec_int4){0x80000000,0x80000000,0x80000000,0}) ) );```
miigotu commented 4 years ago

You can add -Wno-narrowing to the cpp flags in that makefile.

It should work fine, but we should also explicitly static_cast that line or update the include.

zeldin commented 4 years ago

Is the value supposed to be 2147483648? In that case I guess the code should simply say (vec_uint4){0x80000000U,0x80000000U,0x80000000U,0} instead?

ParzivalWolfram commented 4 years ago

I'm unsure, but from the surrounding code, it seems 0x7FFFFFFF would be sufficient. I'm not the greatest at C, however, so take someone else's word for it, not mine.

On Thu, May 7, 2020 at 3:11 PM Marcus Comstedt notifications@github.com wrote:

Is the value supposed to be 2147483648? In that case I guess the code should simply say (vec_uint4){0x80000000U,0x80000000U,0x80000000U,0} instead?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ps3dev/PSL1GHT/issues/70#issuecomment-625471966, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHE6CQNZNZZ5MVRPXS34X5DRQMIWRANCNFSM4M3ELFNQ .

zeldin commented 4 years ago

Since the value is an operand to XOR, using 0x7FFFFFFF would give a very different result I think. The intention seems to be to invert the sign bit, not every bit except the sign bit. :smile_cat:

zeldin commented 4 years ago

Come to think of it, maybe the whole thing can be rewritten to return Quat( vec_xor( quat.get128(), ((vec_float4){-0.0, -0.0, -0.0, 0.0}))); without involving any integers at all?

ParzivalWolfram commented 4 years ago

I didn't catch that that was the intent, like I said, i'm not great at C... hence the disclaimer.

On Thursday, May 7, 2020, Marcus Comstedt notifications@github.com wrote:

Come to think of it, maybe the whole thing can be rewritten to return Quat( vec_xor( quat.get128(), ((vec_float4){-0.0, -0.0, -0.0, 0.0}))); without involving any integers at all?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ps3dev/PSL1GHT/issues/70#issuecomment-625479389, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHE6CQJUWX4T2DMSQINLKI3RQMKRPANCNFSM4M3ELFNQ .

miigotu commented 4 years ago

Pretty sure just changing the cast to vec_uint4 is enough:

return Quat( vec_xor( quat.get128(), ((vec_float4)(vec_uint4){0x80000000,0x80000000,0x80000000,0}) ) );

I don't have the toolchain/libraries set up to test that out though

bucanero commented 4 years ago

I've got a similar problem trying to build the rsxtest sample from PSL1GHT ( https://github.com/ps3dev/PSL1GHT/tree/master/samples/graphics/rsxtest )

by running make, I get the same error about the narrowing conversion.

Following @miigotu comment, I tried casting to vec_uint4 (modified line 502 in /usr/local/ps3dev/ppu/include/vectormath/cpp/quat_aos.h )

Tried to compile again, and to my surprise, I've got a very weird error (at least for my limited knowledge):

% make
main.cpp
/Users/dparrino/github/psl1ght-libs/samples/psl1ght/graphics/rsxtest/source/main.cpp: In function 'int main(int, const char**)':
/Users/dparrino/github/psl1ght-libs/samples/psl1ght/graphics/rsxtest/source/main.cpp:556:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
  s32 ret,i;
      ^~~
/Users/dparrino/github/psl1ght-libs/samples/psl1ght/graphics/rsxtest/source/main.cpp: In function 'void program_exit_callback()':
/Users/dparrino/github/psl1ght-libs/samples/psl1ght/graphics/rsxtest/source/main.cpp:410:1: internal compiler error: in rs6000_savres_routine_name, at config/rs6000/rs6000.c:28661
 }
 ^
libbacktrace could not find executable to open
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://gcc.gnu.org/bugs/> for instructions.
make[1]: *** [main.o] Error 1
make: *** [build] Error 2

did I truly broke gcc, or I did something wrong on my side?

thanks for the help

miigotu commented 4 years ago

@bucanero the first error is easily worked around, by passing -Wno-unused-but-set-variable as a cflag.

bucanero commented 4 years ago

@bucanero the first error is easily worked around, by passing -Wno-unused-but-set-variable as a cflag.

oh yes, the unused variable is not a big deal, what I have no clue about is the internal compiler error 😕

/Users/dparrino/github/psl1ght-libs/samples/psl1ght/graphics/rsxtest/source/main.cpp:410:1: internal compiler error: in rs6000_savres_routine_name, at config/rs6000/rs6000.c:28661
 }
 ^
libbacktrace could not find executable to open
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://gcc.gnu.org/bugs/> for instructions.

it's the first time I see gcc telling me to submit a bug report . I just wanted to check if it was a problem on my side , or someone else also got this issue when trying to build rsxtest

miigotu commented 4 years ago

Ideally someone should just update vectormath in PSL1GHT In the updated library, there is a commit that fixes this same error: https://github.com/erwincoumans/sce_vectormath/commit/5cd9fd0fe73d83cc80479ec07e37d8f6a7ff00e0

Maybe we should replace vectormath from here: https://github.com/erwincoumans/sce_vectormath/tree/master/include/vectormath

miigotu commented 4 years ago

@bucanero can you try checking out the bug/issue-70 branch and compile that?

bucanero commented 4 years ago

@miigotu sure, I'll check out and try to compile. 👍

I'll report back with my results.

bucanero commented 4 years ago

Sorry for the delay, using the fix from bug/issue-70 removed the narrowing conversion error, but the example still fails to compile.

I'll try #77 next.

Edit: at least from my tests, after applying patch from bug/issue-70 and removing the -Os compiler flag from the Makefile, everything worked. I could build the rsxtest example, and execute it correctly on my PS3.