python-greenlet / greenlet

Lightweight in-process concurrent programming
Other
1.63k stars 247 forks source link

Add support for SuperH #398

Open pietro opened 6 months ago

pietro commented 6 months ago

Based on the SH4 Software Manual and on the SH-4 generic and C specific application binary interface.

Untested because i currently don't have access to a SH4 board running Linux and I can't install python in a Debian qemu chroot.

Fixes #166.

glaubitz commented 6 months ago

Tested on my SH7785LCR board. While it compiles without any problems, the testsuite actually segfaults:

I: pybuild base:305: cd /build/python-greenlet-SfE6Y1/python-greenlet-3.0.1/.pybuild/cpython3_3.12/build; python3.12 -m unittest discover -v 
test_break_ctxvars (greenlet.tests.test_contextvars.ContextVarsTests.test_break_ctxvars) ... Segmentation fault
E: pybuild pybuild:391: test: plugin distutils failed with: exit code=139: cd /build/python-greenlet-SfE6Y1/python-greenlet-3.0.1/.pybuild/cpython3_3.12/build; python3.12 -m unittest discover -v 
I: pybuild base:305: cd /build/python-greenlet-SfE6Y1/python-greenlet-3.0.1/.pybuild/cpython3_3.11/build; python3.11 -m unittest discover -v 
test_break_ctxvars (greenlet.tests.test_contextvars.ContextVarsTests.test_break_ctxvars) ... Illegal instruction
E: pybuild pybuild:391: test: plugin distutils failed with: exit code=132: cd /build/python-greenlet-SfE6Y1/python-greenlet-3.0.1/.pybuild/cpython3_3.11/build; python3.11 -m unittest discover -v

Note, I applied the patch against version 3.0.1. I did not test on master.

glaubitz commented 5 months ago

With the suggested changes above, all but the following tests pass.

test_unhandled_exception_in_greenlet_aborts (greenlet.tests.test_cpp.CPPTests.test_unhandled_exception_in_greenlet_aborts) ... FAIL
test_unhandled_nonstd_exception_aborts (greenlet.tests.test_cpp.CPPTests.test_unhandled_nonstd_exception_aborts) ... FAIL
test_unhandled_std_exception_aborts (greenlet.tests.test_cpp.CPPTests.test_unhandled_std_exception_aborts) ... FAIL
test_unhandled_std_exception_as_greenlet_function_aborts (greenlet.tests.test_cpp.CPPTests.test_unhandled_std_exception_as_greenlet_function_aborts) ... FAIL
(...)
test_failed_to_slp_switch_into_running (greenlet.tests.test_greenlet.TestBrokenGreenlets.test_failed_to_slp_switch_into_running) ... FAIL
(...)
test_throw_to_dead_thread_doesnt_crash_wait (greenlet.tests.test_greenlet.TestGreenlet.test_throw_to_dead_thread_doesnt_crash_wait) ... Time limit exceeded.
Threads: Waiting for only 1 --> 1
MGlets : Waiting for only 1 --> 2

However, I'm seeing those failures on SPARC as well, so I assume these are unrelated.

glaubitz commented 5 months ago

Also, I would prefer making the code sub-architecture-agnostic, i.e. available for all SuperH CPUs:

diff --git a/src/greenlet/slp_platformselect.h b/src/greenlet/slp_platformselect.h
index 0099efc..6096cb5 100644
--- a/src/greenlet/slp_platformselect.h
+++ b/src/greenlet/slp_platformselect.h
@@ -64,8 +64,8 @@ extern "C" {
 # include "platform/switch_aarch64_gcc.h" /* LLVM Aarch64 ABI for Windows */
 #elif defined(__GNUC__) && defined(__loongarch64) && defined(__linux__)
 # include "platform/switch_loongarch64_linux.h" /* LoongArch64 */
-#elif defined(__GNUC__) && defined(__SH4__)
-# include "platform/switch_sh4_gcc.h" /* SH4 */
+#elif defined(__GNUC__) && defined(__sh__)
+# include "platform/switch_sh_gcc.h" /* SH */
 #endif

 #ifdef __cplusplus

and rename the header accordingly:

src/greenlet/platform/switch_sh4_gcc.h -> src/greenlet/platform/switch_sh_gcc.h

and change the architecture name in the commit message from "SH4" to "SH".

glaubitz commented 5 months ago

I have incorporated all changes here now: https://github.com/glaubitz/greenlet/commit/e73592dec7950ad9fcdb5c0287ef3758df010f11

Let me know if you want me to open a PR. You are still being credited as the author, of course.

glaubitz commented 5 months ago

I have fixed your patch and resubmitted as https://github.com/python-greenlet/greenlet/pull/401 while keeping you as the author.

pietro commented 5 months ago

I have fixed your patch and resubmitted as #401 while keeping you as the author.

Thanks. I didn't see you had opened a PR so I updated my branch with your suggestions.

glaubitz commented 5 months ago

Can you change your commit message to:

Add support for SuperH

Fixes #166

I think the additional "CPUs" is not needed and the fixes tag is useful as it will automatically close the issue.