irixxxx / picodrive

Fast MegaDrive/MegaCD/32X emulator
Other
55 stars 25 forks source link

Add dynarec support for PS3 #82

Closed OsirizX closed 1 year ago

OsirizX commented 2 years ago

Some minor changes were needed for emit_ppc.c to get dynarec working on the ps3. We need to force load TOC and function address from opd section in emith_call_reg and emith_abijump_reg otherwise a crash would occur.

irixxxx commented 1 year ago

Could you please explain what the change in compiler.c is exactly doing? It looks a bit strange to me.

OsirizX commented 1 year ago

Yeah function pointers on the ps3 are a bit odd. I believe you ran into the same issue I did initially where it would load the value the pointer points to and jumps to it. This resulted in a crash. The workaround is to set another pointer to it so that it jumps to the address of sh2_drc_entry instead.

https://github.com/libretro/picodrive/issues/150#issuecomment-830696067

irixxxx commented 1 year ago

Hmm, drc_entry is defined as a function pointer, so if the compiler is handling this correctly it should probably reserve the needed 8 bytes for the indirection.

The interesting question is why this is not so. Is this possibly a compiler problem? If so and if it is ever fixed in there, the additional indirection code in your patch would fail. At the very least it should be moved into the emit file since it's platform specific. Please point me to the toolchain. I'll conduct some additional testing with it and see if I can come up with a cleaner way to handle this.

irixxxx commented 1 year ago

I believe there is no general solution to this since the C standards don't define what exactly a function pointer is. It's normally a code address, but that's not a given. Nonetheless I suggest burying the ps3 dependencies in the ppc code emitter to have them in one place. I'll devise something since it has to be done that way in all the code emitters.

irixxxx commented 1 year ago

I finally came around to do the necessary changes yesterday. I introduced a new macro to convert a memory pointer to a function pointer for calling by the host. I also took care to mark all emitter macros which call host functions by abicall. Would you please adapt your patch to this before merging?

OsirizX commented 1 year ago

Thanks @irixxxx for making the changes. Makes sense to use the array of 2 ptrs due to the function descriptor defined for the ps3 below. Can also let me know if any other changes are needed for my commit

typedef struct function_descriptor
{
    uint64_t addr;
    uint64_t toc;
}f_desc_t;

Reference

irixxxx commented 1 year ago

Could you please import this small cleanup patch? It confines the changes for ABI calling in the abicall macros.

x.txt

I'm wondering if PS3 really needs assigning the call target to CR. That might be be an ELF ABI specific thing. You might want to try to remove the 2 if ((r) != CR)... lines from the PS3 versions and see if that still runs. If so it might infinitesimally improve performance. That's however up to you.

OsirizX commented 1 year ago

I've applied the cleanup patch and used PTR_SIZE instead of hard coding it when reading the TOC. The size changes depending on which ps3 sdk is used (4 w/ official vs 8 w/ psl1ght). Also I tried removing the 2 lines but unfortunately it crashes upon game boot so leaving them in for now.

irixxxx commented 1 year ago

Super, thank you very much for your work!

irixxxx commented 1 year ago

I've merged this into lr-picodrive right away outside of my normal merge window.

irixxxx commented 1 year ago

Sorry to bother you once more, but could you possibly try this patch?

x.txt

The committed change works, but only because abicall_reg is always used with CR as parameter. This patch should correct this behaviour.

OsirizX commented 1 year ago

I can confirm the patch works great! No crashes with this one. Thanks!