Skip to content

riscv64 support for fibers #7879

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

Closed
wants to merge 2 commits into from
Closed

riscv64 support for fibers #7879

wants to merge 2 commits into from

Conversation

jcourreges
Copy link
Contributor

The current PHP codebase omits the riscv64 assembly files for make_fcontext/jump_fcontext from Boost.Context. Sync those from boostorg/context and hook them up.

@jcourreges
Copy link
Contributor Author

This change has already been committed to the OpenBSD ports tree.

Basic testing was done on OpenBSD/riscv64 using the php-src git repo master branch:

hifive ~/src/php-src$ sapi/cli/php run-tests.php Zend/tests/fibers/  
                                                                               
=====================================================================    
PHP         : /home/test-user/src/php-src/sapi/cli/php                                                                                                         
PHP_SAPI    : cli                                          
PHP_VERSION : 8.2.0-dev                                             
ZEND_VERSION: 4.2.0-dev                                                    
PHP_OS      : OpenBSD - OpenBSD hifive.wxcvbn.org 7.0 GENERIC.MP#8 riscv64                                                                                     
INI actual  : /home/test-user/src/php-src                                  
More .INIs  :                                                                                                                                                  
---------------------------------------------------------------------                                                                                          
PHP         : /home/test-user/src/php-src/sapi/cgi/php-cgi                                                                                                     
PHP_SAPI    : cgi-fcgi                                             
PHP_VERSION : 8.2.0-dev                                                                                                                                        
ZEND_VERSION: 4.2.0-dev                                        
PHP_OS      : OpenBSD - OpenBSD hifive.wxcvbn.org 7.0 GENERIC.MP#8 riscv64                                                                                     
INI actual  : /home/test-user/src/php-src                                                                                                                      
More .INIs  :                                                                                                                                                  
---------------------------------------------------------------------                                                                                          
---------------------------------------------------------------------                                                                                          
PHP         : /home/test-user/src/php-src/sapi/phpdbg/phpdbg                                                                                                   
PHP_SAPI    : phpdbg                                            
PHP_VERSION : 8.2.0-dev                                                                                                                                        
ZEND_VERSION: 4.2.0-dev                                                                                                                                        
PHP_OS      : OpenBSD - OpenBSD hifive.wxcvbn.org 7.0 GENERIC.MP#8 riscv64                                                                                     
INI actual  : /home/test-user/src/php-src                                                                                                                      
More .INIs  :                                                                                                                                                  
---------------------------------------------------------------------                                                                                          
CWD         : /home/test-user/src/php-src                                                                                                                      
Extra dirs  :                                                                
VALGRIND    : Not used                                                                                                                                         
=====================================================================                                                                                          
Running selected tests.                                                                                                                                        
PASS Backtrace in deeply nested function call [Zend/tests/fibers/backtrace-deep-nesting.phpt]
PASS Backtrace in nested function call [Zend/tests/fibers/backtrace-nested.phpt]
PASS Backtrace in with object as fiber callback [Zend/tests/fibers/backtrace-object.phpt]
PASS Catch exception thrown into fiber, then suspend again [Zend/tests/fibers/catch-then-suspend.phpt]
PASS Catch exception thrown into fiber [Zend/tests/fibers/catch.phpt]                                                                                          
PASS Print backtrace in fiber [Zend/tests/fibers/debug-backtrace.phpt]                                                                                         
PASS Start on already running fiber [Zend/tests/fibers/double-start.phpt]                                                                                      
PASS Error reporting change reflected inside fiber [Zend/tests/fibers/error-reporting.phpt]
PASS Exit from fiber [Zend/tests/fibers/exit-in-fiber.phpt]                                                                                                    
PASS Test throwing from fiber [Zend/tests/fibers/failing-fiber.phpt]                                                                                           
PASS Test throwing from fiber [Zend/tests/fibers/failing-nested-fiber.phpt]                                                                                    
PASS Fast finishing fiber does not leak [Zend/tests/fibers/fast-finish-fiber.phpt]          
PASS Fatal error in new fiber [Zend/tests/fibers/fatal-error-in-fiber.phpt]
PASS Fatal error within a nested fiber [Zend/tests/fibers/fatal-error-in-nested-fiber.phpt]                                    
PASS Fatal error in a fiber with other active fibers [Zend/tests/fibers/fatal-error-with-multiple-fibers.phpt]                   
PASS FiberError cannot be constructed in user code [Zend/tests/fibers/fiber-error-construct.phpt]
PASS Fiber::getCurrent() [Zend/tests/fibers/fiber-get-current.phpt]
PASS Fiber in shutdown function [Zend/tests/fibers/fiber-in-shutdown-function.phpt]                 
PASS Fiber status methods [Zend/tests/fibers/fiber-status.phpt]                                                                                                
PASS GC can cleanup cycle when callback references fiber [Zend/tests/fibers/gc-cycle-callback.phpt]                 
PASS GC can cleanup cycle when fiber result references fiber [Zend/tests/fibers/gc-cycle-result.phpt]             
PASS Fiber::getReturn() after bailout [Zend/tests/fibers/get-return-after-bailout.phpt]
PASS Fiber::getReturn() after a fiber throws [Zend/tests/fibers/get-return-after-throwing.phpt]
PASS Fiber::getReturn() from unstarted fiber [Zend/tests/fibers/get-return-from-unstarted-fiber.phpt]
PASS Fiber::getReturn() in unfinished fiber [Zend/tests/fibers/get-return-in-unfinished-fiber.phpt]
PASS Test fiber return value [Zend/tests/fibers/get-return.phpt]
PASS Reference to invocable class retained while running [Zend/tests/fibers/invocable-class.phpt]
PASS Cannot resume fiber within destructor [Zend/tests/fibers/no-switch-dtor-resume.phpt]
PASS Cannot start fiber within destructor [Zend/tests/fibers/no-switch-dtor-start.phpt]
PASS Cannot suspend fiber within destructor [Zend/tests/fibers/no-switch-dtor-suspend.phpt]
PASS Cannot resume fiber within destructor [Zend/tests/fibers/no-switch-dtor-throw.phpt]
PASS Cannot start a new fiber in a finally block in a force-closed fiber [Zend/tests/fibers/no-switch-force-close-finally.phpt]
PASS Context switches are prevented during GC collect cycles [Zend/tests/fibers/no-switch-gc.phpt]
PASS Out of Memory in a fiber [Zend/tests/fibers/out-of-memory-in-fiber.phpt]
PASS Out of Memory in a nested fiber [Zend/tests/fibers/out-of-memory-in-nested-fiber.phpt]                                                                    
PASS Out of Memory from recursive fiber creation [Zend/tests/fibers/out-of-memory-in-recursive-fiber.phpt]                                                     
PASS Resume non-running fiber [Zend/tests/fibers/resume-non-running-fiber.phpt]                                                                                
PASS Resume previous fiber [Zend/tests/fibers/resume-previous-fiber.phpt]                                                                                      
PASS Resume running fiber [Zend/tests/fibers/resume-running-fiber.phpt]                                                                                        
PASS Resume terminated fiber [Zend/tests/fibers/resume-terminated-fiber.phpt]                                                                                  
PASS Test resume [Zend/tests/fibers/resume.phpt]                                                                                                               
PASS Fiber function may return by ref, but getReturn() always returns by val [Zend/tests/fibers/return-by-ref.phpt]                                            
PASS Silence operator does not leak out of fiber [Zend/tests/fibers/silence-operator-inside-fiber.phpt]                                                        
PASS Silence operator does not leak into fiber [Zend/tests/fibers/silence-operator-outside-fiber.phpt]                                                         
PASS Arguments to fiber callback [Zend/tests/fibers/start-arguments.phpt]                                                                                      
PASS Suspend in force-closed fiber after shutdown [Zend/tests/fibers/suspend-in-force-close-fiber-after-shutdown.phpt]                                         
PASS Suspend in force-closed fiber, catching exception thrown from destructor [Zend/tests/fibers/suspend-in-force-close-fiber-catching-exception.phpt]         
PASS Suspend in force-closed fiber [Zend/tests/fibers/suspend-in-force-close-fiber.phpt]                                                                       
PASS Suspend within nested function call [Zend/tests/fibers/suspend-in-nested-function.phpt]
PASS Suspend outside fiber [Zend/tests/fibers/suspend-outside-fiber.phpt]  
PASS Make sure exceptions are rethrown when throwing from fiber destructor [Zend/tests/fibers/throw-during-fiber-destruct.phpt]
PASS Throw in multiple destroyed fibers after shutdown [Zend/tests/fibers/throw-in-multiple-destroyed-fibers-after-shutdown.phpt]
PASS Throw into non-running fiber [Zend/tests/fibers/throw-into-non-running-fiber.phpt]          
PASS Test throwing into fiber [Zend/tests/fibers/throw.phpt]       
PASS Test unfinished fiber with finally block [Zend/tests/fibers/unfinished-fiber-with-finally.phpt]
PASS Test unfinished fiber with nested try/catch blocks [Zend/tests/fibers/unfinished-fiber-with-nested-try-catch.phpt]                                        
PASS Test unfinished fiber with suspend in finally [Zend/tests/fibers/unfinished-fiber-with-suspend-in-finally.phpt]
PASS Test unfinished fiber with suspend in finally [Zend/tests/fibers/unfinished-fiber-with-throw-in-finally.phpt]
PASS Test unfinished fiber [Zend/tests/fibers/unfinished-fiber.phpt]                                                                                           
PASS Not starting a fiber does not leak [Zend/tests/fibers/unstarted-fiber.phpt]               
=====================================================================                                                                                          
Number of tests :   60                60                                                                                                                       
Tests skipped   :    0 (  0.0%) --------                        
Tests warned    :    0 (  0.0%) (  0.0%)                                                                                                                       
Tests failed    :    0 (  0.0%) (  0.0%)                                                                                                                       
Tests passed    :   60 (100.0%) (100.0%)                                                                                                                       
---------------------------------------------------------------------                                                                                          
Time taken      :   11 seconds                                                                                                                                 
=====================================================================                                                                                          
hifive ~/src/php-src$                                                                                                                                          

@jcourreges
Copy link
Contributor Author

While I'm proposing a merge against the master branch, it would be nice to also have this change included on the PHP-8.1 stable branch. That would get us (OpenBSD) rid of patches, and would also let people build php-8.1 on riscv64 systems without ucontext_t support as expected by configure.ac.

@sthen
Copy link

sthen commented Jan 3, 2022

(note that openbsd does not and likely will never have ucontext)

@cmb69
Copy link
Member

cmb69 commented Jan 3, 2022

The current PHP codebase omits the riscv64 assembly files for make_fcontext/jump_fcontext from Boost.Context.

@trowski, have there been any particular reasons to omit these files?

@trowski
Copy link
Member

trowski commented Jan 3, 2022

@cmb69 Only the files that could possibly be used by configure have been bundled. There's no reason to exclude any of the asm files once they've been added to configure and tested. It's nice to see these platforms being tested.

If we merge #7623, then all the Boost.Context asm files would be bundled, however more changes will be necessary for configure to resolve to those bundled files.

This should be merged into the PHP-8.1 branch IMO.

@cmb69
Copy link
Member

cmb69 commented Jan 4, 2022

This should be merged into the PHP-8.1 branch IMO.

Unless RMs object – @krakjoe, @ramsey, @patrickallaert.

@ramsey
Copy link
Member

ramsey commented Jan 4, 2022

No objection from me.

@krakjoe
Copy link
Member

krakjoe commented Jan 4, 2022

This looks fine to merge into 8.1.

At this point we should either merge or close #7623.

For me, I would rather only bundle what configure may include in a build.

Someone with knowledge of and access to the remaining (exotic) platforms needs to do the same sort of legwork as was done here. Bundling a bunch of assembly we can't use is a little strange.

@cmb69
Copy link
Member

cmb69 commented Jan 4, 2022

I think it makes sense to merge this PR now, so it can be in PHP 8.1.2RC1 (which is supposed to be tagged today). We can still have a closer look at the other PR afterwards.

@cmb69 cmb69 closed this in 70b02d7 Jan 4, 2022
@andypost
Copy link
Contributor

andypost commented Jan 7, 2022

Thank you! it works great, one more dependency could be removed (Alpinelinux used libucontext for rv64)

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.

7 participants