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

JIT for PHP based on DynAsm #3792

Open
wants to merge 870 commits into
base: master
from

Conversation

7 participants
@dstogov
Copy link
Contributor

dstogov commented Feb 4, 2019

No description provided.

dstogov and others added some commits May 25, 2017

Merge branch 'master' into jit-dynasm
* master:
  Turn interactive mode on, if cli binary is clicked in filemanager
  Fixed copy-paste mistake
  Added ZEND_GET_CLASS, ZEMD_GET_CALLED_CLASS, ZEND_GET_TYPE instructions, to implement corresponding builtin functions.
  Added a test case for (docs) bug #74652
  Fixed test (backtrace was changed)
  Added ZEND_COUNT instruction, to implement corresponding builtin.
  "Countable" interface is moved from SPL to Core
  Fixed ZEND_IN_ARRAY related issues
  Improved UTF-8 validation in JSON
  Remove `user=foo` from FPM test config
  Skip FPM tests when running as root (unless requested)
Merge branch 'jit-dynasm' of github.com:zendtech/php-src into jit-dynasm
* 'jit-dynasm' of github.com:zendtech/php-src:
  Use LVAL directly
Merge branch 'master' into jit-dynasm
* master: (89 commits)
  Replace ASN1_STRING_data with ASN1_STRING_get0_data
  Fix leak in WDDX serialization
  Travis: Use opcache in release build
  Implemented FR #71520
  Fixed bug #69373
  Fix bug #74607: Don't check for bi-directional compatibility in traits
  Fix bug #55407
  Fixed bug #73473: Stack Buffer Overflow in msgfmt_parse_message
  openssl_pkcs12_read: add missing BIO_free
  Add basic test for posix_setegid function
  Set timezone for intl/test/bug74298.phpt
  Revert "merge PR #2290: enable opcache in CLI in 7.1+"
  PCRE_EXTRA_MARK is useful only for preg_replace_callbakc(). Removed branch expectations.
  Added support for PCRE JIT fast path API
  Escape value passed to exec()
  Ignore spurious stderr output from lsof
  ZVAL_COPY_UNREF() micro-optimization
  test for ErrorException::getSeverity();
  improve dns (checkdnsrr) test coverage
  test to function forward_static_call_array();
  ...
Merge branch 'master' into jit-dynasm
* master: (60 commits)
  VM refactoring, to avoid passing "execute_data" into helper functions that can access it using global register variable.
  follow up on 0c992792220bbfb375d5dc8222beb2a55da8441a
  Fixed php_socket_t to int conversion
  fix ticks init in ts build
  Refactored API for constant array element propagation
  code de-duplication in ReflectionType::__toString and ReflectionNamedType::getName
  Moved "Using $this when not in object context" exception code into single VM helper.
  Change PHP_OS_FAMILY to "Darwin" instead of "Mac" for Darwin based systems (as suggested by Davey)
  Fixed bug #74679 (Incorrect conversion array with WSDL_CACHE_MEMORY)
  Use "Mac" instead of "OSX" to identify macOS in PHP_OS_FAMILY
  Avoid run-time checks performed at compile-time.
  Fixed performance degradaton introduced in f6ac96b
  NEWS
  NEWS
  Fix bug #74705 for collator_get_sort_key
  Fixes bug #74705 Wrong ReflectionInfo for Collator::getSortKey()
  NEWS
  NEWS
  Fixes bug #74708 reflection signatures for random_bytes+random_int
  Avoiding str duplication
  ...
Merge branch 'master' into jit-dynasm
* master:
  Fixed wrong usage of old ZPP API.
  Avoid useless dereferences and separations during paramter passing.
  Optimization for fast path.
  NEWS
  Default single_dh_use and honor_cipher_order to true
  NEWS
  Compatibility with libargon2 versions 20161029 and 20160821
  add test for strptime(): return false on failure
  openssl: Add openssl_pkcs12_(read, export) tests
  Regenerate and bump re2c version to 0.16
  Next round on AppVeyor reliability.
  openssl: Fix spkstr and spki leak in openssl_spki_new
  Change flags to use SQLITE3_OPEN_READ* constants instead of a fake-boolean, add tests on errors
  Implement writing to BLOBs in SQLite3
Merge branch 'master' into jit-dynasm
* master: (302 commits)
  Tweak
  Added goblal optimisation passes based on data flow analyses using SSA form: SCCP - Sparse Conditional Constant Propagation, DCE - Dead Code Elimination and removing of unused local variablesi.
  Fixed bug #74923 Crash when crawling through network share
  Bump OCI8 version for recent patch
  WS
  Fix test title
  Ensure that the stream position is kept between reads
  Turn off EXIF_DEBUG so Travis don't complain at me
  Don't add a new line to undefined tags in EXIF_DEBUG mode
  Fix compile error with EXIF_DEBUG
  update NEWS
  disable --with-pcre-valgrind on travis
  fix default args for --with-pcre-valgrind
  Enable valgrind support for PCRE by default in debug builds
  add oniguruma.patch to ease future upgrades
  SIZEOF_SIZE_T doesn't exist on AIX and POWER8 (ppc64le), keep using SIZEOF_LONG
  fix fold
  Fixed bug #74866 extension_dir = "./ext" now use current directory for base
  add next vc15 toolset to the list
  Revert "Enable whole program optimization for builds without PGO, too"
  ...
Merge branch 'master' into jit-dynasm
* master: (22 commits)
  Fixed CFG/SSA construction (avoid multiple identical predecessors)
  Removed vim mode lines. zend_vm_opcodes.h loses these lines after regeneration. Lines in zend_vm_def.h lead to insertion inthe middle of zend_vm_execute.h.
  Allocate additional slot for third argument
  fix new password related pieces wrt failing tests
  Fixed bug #74906 redirecting incorrect include <sys/errno.h>
  Fixed bug #74906 redirecting incorrect include <sys/errno.h>
  Fixed bug 74913 redirecting incorrect include <sys/poll.h>
  fix typo
  fix missing var for phpize
  Refactor password_hash()
  Refactor password.c
  Provide zend_string wrappers for php_base64_(en|de)code
  Make functions in openssl.c more consistent
  Make consitent naming and improve CS in xp_ssl
  Allow setting SNI cert and pk in separate files
  Bump PHP_JSON_VERSION to 1.6.0
  [ci skip] Remove CSV leftovers from json code - $Id$
  Introduce internal php_json_encode_ex to allow extensions setting depth
  Add JSON_INVALID_UTF8_SUBSTITUTE and JSON_INVALID_UTF8_IGNORE
  Add test for bug #74923
  ...
Merge branch 'master' into jit-dynasm
* master: (48 commits)
  Fixed bug #74941 - Session fails to start after having headers sent
  Directly accept encoding in php_unicode_convert_case()
  Add php_mb_get_encoding() convenience function
  Optimize php_unicode_is_prop()
  Avoid unnecessary encoding lookups in mbstring
  fix dir separator in test
  Do not allow using traits/interfaces/abstract classes as stream wrappers
  Fixed bug #74851: Improve uniqid() performance
  Remove version checks for MySQL < 5.0
  Add oci8_failover.c to config.w32, follow up for commit 1b797f7
  Keep resource of enclosing stream, because it may be referenced from other place(s). This fixes valgrind warnings on Zend/tests/type_declarations/scalar_basic.phpt
  Separate the fast-patch
  Convert CONCAT into FAST_CONCAT for non-object operands
  Reset globals on startup or restart
  Remove dead live ranges and FREE instructions
  Remove live ranges
  Removed Bird(broken)step support from ODBC
  Remove old references to SAPIs and extensions no longer in the core
  Don't reuse compare_function operands
  Add notes for cutting release branches
  ...
Merge branch 'master' into jit-dynasm
* master: (238 commits)
  Update NEWs
  Update NEWS
  Fixed bug #75049 (spl_autoload_unregister can't handle spl_autoload_functions results)
  Remove mistakingly added line
  Sync makefile options for phpize
  sodium ext: Use _ietf_ vs _IETF_ consistently
  sodium ext: No need for #ifdef crypto_aead_chacha20poly1305_IETF_
  Sodium ext: Isolate a return statement for consistency
  sodium ext: The default password hashing function is not supposed to be Argon2i
  sodium ext: long -> zend_long
  sodium ext: Add missing "return" statements after zend_throw_exception()
  JSON: fix config.w32 / Install headers on windows
  Add new enum options defined in MySQL 5.7
  Fixed compiler warning(enumeration value ‘SPL_FS_INFO’ not handled in switch)
  Unify EOL
  fix test target for phpize
  Regenerate by re2c 0.16
  Fix segfault in json ignoring of invalid UTF8
  Refactor php_url struct to save memory dup in common cases
  Optimized ucfirst(Avoid duping string if possible)
  ...
Merge branch 'jit-dynasm' of github.com:zendtech/php-src into jit-dynasm
* 'jit-dynasm' of github.com:zendtech/php-src:
  Fixed error message
Merge branch 'master' into jit-dynasm
* master:
  Order live ranges according to "start" position
  Fixed attempt to free invalid structure (result of ROPE_INIT is not a zval)
  Revert "Fixed live_range removing (bug can be triggred by JIT)"
  Revert "Fixed live_range removing (bug can be triggred by JIT)"
  Sync OCI8 on PHP 7.x branches
  Fixed bug #75063
  Fixed bug #73793 (WDDX uses wrong decimal seperator)
  Skip this test if ext/session is not available
  Note deprecation of $errcontext
  Fixed bug #74103 and bug #75054
  Fix bug #74725 (html_errors=1 breaks unhandled exceptions)
  Change getimagesize() and friends to report image/bmp
Merge branch 'master' into jit-dynasm
* master:
  Fixed removing dead live ranges
  [ci skip] update NEWS
  [ci skip] update NEWS
Merge branch 'master' into jit-dynasm
* master:
  Test cleanup improvements, files might be locked in the test process
  fix test cleanup
Show resolved Hide resolved Zend/zend_gdb.c Outdated
Show resolved Hide resolved ext/opcache/jit/zend_elf.c Outdated
Show resolved Hide resolved ext/opcache/jit/zend_jit.h Outdated

dstogov added some commits Feb 5, 2019

Merge branch 'master' into jit-dynasm
* master:
  Fix type confusion
  fixed typo in UPGRADING.INTERNALS
  Removed ldap_sort and LDAP_DEPRECATED build flag
  Mark ldap_control_paged_result and ldap_control_paged_result_response as deprecated
  Reset common fields of EG(trampoline)
  Added note anout object habdlers API change
Merge branch 'master' into jit-dynasm
* master:
  Remove copyright years.
return NULL;
}

static ZEND_COLD void zend_jit_throw_incdec_ref_error(zend_reference *ref, zend_bool inc)

This comment has been minimized.

@nikic

nikic Feb 6, 2019

Member

Do I understand right that most of the code in this file can be deduplicated by exporting symbols from zend_execute.c and the current implementation is to avoid changing Zend code?

This comment has been minimized.

@dstogov

dstogov Feb 6, 2019

Author Contributor

yes. Of course. JIT was developed as a completely independent part, and I didn't try to made a lot of related changes in the main-stram engine. Once, they are started developing together, we should solve this code duplication issue.

I think, this is not a problem at all.

Show resolved Hide resolved ext/opcache/jit/zend_jit.c Outdated
| vcvtsi2sd, xmm(reg-ZREG_XMM0), xmm(reg-ZREG_XMM0), r0
|| } else {
| cvtsi2sd, xmm(reg-ZREG_XMM0), r0
|| }

This comment has been minimized.

@nikic

nikic Feb 6, 2019

Member

Wouldn't it be better to load the double representation from memory? I believe cvtsi2sd has quite high latency.

Does dynasm have any way to place constant data near the function?

This comment has been minimized.

@dstogov

dstogov Feb 6, 2019

Author Contributor

I'm not sure what is worse int->double conversion or loading from memory.
Mixing code and data in one cache line is also bad.
We may put data into .slow_code section, or better introduce another .ro_data section.

|| }
| FPU_SET_ZVAL_DVAL dst_addr
| .endif
|| } else if (Z_LVAL_P(zv) == 0 && Z_MODE(dst_addr) == IS_REG) {

This comment has been minimized.

@nikic

nikic Feb 6, 2019

Member

To double check, LVAL is used here also for the address of things like zend_string/zend_array, right?

This comment has been minimized.

@dstogov

dstogov Feb 6, 2019

Author Contributor

Yes, however, Z_STR or Z_ARR shouldn't be NULL.

Show resolved Hide resolved ext/opcache/jit/zend_jit_x86.dasc Outdated
|| } while(0);
|.endmacro

|.macro ZVAL_PTR_DTOR, addr, op_info, gc, cold, safe, opline

This comment has been minimized.

@nikic

nikic Feb 6, 2019

Member

A comment for what the gc and safe flags do would be helpful here.

|| } else if (cold && ((op_info) & ((MAY_BE_ANY|MAY_BE_UNDEF)-(MAY_BE_OBJECT|MAY_BE_RESOURCE)))) {
| jmp >4
|.code
|| }

This comment has been minimized.

@nikic

nikic Feb 6, 2019

Member

This code is the same at the end of the if and in the else if, it should be fine to move it outside as a single check.

This comment has been minimized.

@nikic

nikic Feb 6, 2019

Member

By the way, I found the implementation of this macro quite confusing, it would probably be a lot more obvious if MAY_BE_N was a completely separate codepath, at the expense of duplicating some of the code.

This comment has been minimized.

@dstogov

dstogov Feb 6, 2019

Author Contributor

Let's delay optimizations until merge

Show resolved Hide resolved ext/opcache/jit/zend_jit_x86.dasc Outdated
|| }
|2:
|| }
| GET_ZVAL_LVAL ZREG_FCARG1a, addr

This comment has been minimized.

@nikic

nikic Feb 6, 2019

Member

Worth pointing out this side-effect in a comment above the macro.

This comment has been minimized.

@dstogov

dstogov Feb 6, 2019

Author Contributor

Probably, it makes sense to add uniform description for all macros.


static uint32_t concrete_type(uint32_t value_type)
{
return floor_log2(value_type & MAY_BE_ANY);

This comment has been minimized.

@nikic

nikic Feb 6, 2019

Member

We should probably add some portability wrappers for __builtin_ctz. We already use __builtin_clz, but not this one.

This comment has been minimized.

@dstogov

dstogov Feb 6, 2019

Author Contributor

This is portable C code, taken from "Hackers Delight" book.
In any case, I agree to add potable bit scanning wrappers.

dstogov added some commits Feb 6, 2019

dstogov added some commits Feb 7, 2019

Merge branch 'master' into jit-dynasm
* master:
  Fix DIM_OBJ specialization in zend_vm_get_opcode_handler_func
  Fix NEWS
  Fix typo
  Remove major version from apache module
  Remove unnecessary setting of error_reporting in intl tests
  Remove $errcontext argument to error handlers
  Some more test removals
  Remove zpp variation tests
Merge branch 'master' into jit-dynasm
* master:
  Remove ZEND_OVERLOADED_FUNCTION and corresponding call_method object handler
  [ci skip] Update changelog
  Fix signedness in comparison in array_slice
  Remove two more zpp variation tests
  Remove --disable-opcache-filecache option
  Use pkg-config for ICU, as the old icu-config has been deprecated
  Require icu-uc and icu-i18n next to icu-io
Merge branch 'master' into jit-dynasm
* master:
  Add ZEND_TRY_ASSIGN_BOOL API
  Fix refcounting of prop types coming from traits
  Add ZEND_TRY_ASSIGN_BOOL API
  Fix invalid free
  Fix SplHeap::compare arginfo and tests
  Fix FTPS passive mode of data channel event poll
  Sync test with changes in libcurl 7.64.0
  Sync test for libcurl 7.64.0
  [ci skip] Move OPcache configure option changes
  Simplify checks
Merge branch 'master' into jit-dynasm
* master: (62 commits)
  Fixed possible crash
  Fixed bug #77599 (Unbuffered queries; native prepared statements memory leak)
  Avoid dependency on "struct flock" fields order.
  Replace broken binary SDK version
  Update SDK version for AppVeyor
  Deprecate ext/wddx
  Fix 32-bit build
  Add missing braces in UPGRADING example
  Don't silence fatal errors with @
  Fix bug #51068 (glob:// do not support current path relative)
  Add bless_tests.php
  Remove oniguruma lines from CONTRIBUTING
  Add UPGRADING notes for oniguruma unbundling
  Unbundle oniguruma in config.w32
  Add MB_ONIGURUMA_VERSION and use it in a version dependent test
  Unbundle oniguruma
  Enable C99 in autoconf
  Validate subject encoding in mb_split and mb_ereg_match
  Validate pattern against mbregex encoding
  Fix #77552: Uninitialized buffer in stat functions
  ...
Merge branch 'master' into jit-dynasm
* master:
  Fixed resoure numbering
  Adding bunch of FreeBSD socket options flags specifics.
  Make pid & uid available while handling realtime signals
  Remove "defensive copy" of DatePeriod properties
  Test mb_str* functions for 'unknown encoding' warnings
  Fixed bug #77564: Memory leak in exif_process_IFD_TAG
| jne >2
|.if X64
| movsxd r1, dword OP:r0->op2.constant
| add r0, r1

This comment has been minimized.

@nikic

nikic Feb 13, 2019

Member

nit: Indentation

| mov r0, IP
| sub r0, aword [r1 + offsetof(zend_op_array, opcodes)]
| // divide by sizeof(zend_op)
| .if X64

This comment has been minimized.

@nikic

nikic Feb 13, 2019

Member

It would be good to ZEND_ASSERT the hardcoded sizes here.

|1:
| shr r0, 3
| mov r1, r0
| .if X64

This comment has been minimized.

@nikic

nikic Feb 13, 2019

Member

This code is not very obvious. I'm assuming that this is generating some kind of hash (which?) and looking it up in a table (ignoring collisions). A comment explaining what this does would be good.

|.define SSE, 1
|.endif

|.define HYBRID_SPAD, 16

This comment has been minimized.

@nikic

nikic Feb 13, 2019

Member

Where do these different stack padding values come from?

This comment has been minimized.

@dstogov

dstogov Feb 13, 2019

Author Contributor

According to x86 and x86_64 ABI, CPU stack has to be 16 byte aligned to guarantee proper alignment of 128-bit SSE data on stack. With broken alignment any execution of SSE code, including calls to memcpy() and others, may lead to crash.

May be, we may avoid paddinh for "Hybrid VM".

static zend_bool delayed_call_chain;
static uint32_t delayed_call_level;
static const zend_op *last_valid_opline;
static int jit_return_label;

This comment has been minimized.

@nikic

nikic Feb 13, 2019

Member

I realize that ZTS support also has other open issues, but we'll probably want to have these in thread local storage, so the JIT can be invoked from multiple threads.

This comment has been minimized.

@dstogov

dstogov Feb 13, 2019

Author Contributor

JIT is executed under opcache "lock", so it's not a problem right now.

last_valid_opline++;

/* Skip the following OP_DATA */
switch (opline->opcode) {

This comment has been minimized.

@nikic

nikic Feb 13, 2019

Member

Rather than hardcoding this list, wouldn't it be more robust and simpler to check if the next opline is OP_DATA?

This comment has been minimized.

@dstogov

dstogov Feb 13, 2019

Author Contributor

The next opcode may be out of op_array->opcodes[] upper boundary...
Anyway, this may make sense.


static int zend_jit_cond_jmp(dasm_State **Dst, const zend_op *next_opline, unsigned int target_label)
{
| cmp IPl, next_opline

This comment has been minimized.

@nikic

nikic Feb 13, 2019

Member

This is comparing only the low bits of the opline on the assumption that it's practically impossible to have both target oplines differing only in the high bits, right?

This comment has been minimized.

@dstogov

dstogov Feb 13, 2019

Author Contributor

right. I'll add comment.

| cmp IPl, next_opline
| je =>next_label
| cmp IPl, target_opline
| je =>target_label

This comment has been minimized.

@nikic

nikic Feb 13, 2019

Member

How can it happen that IP is neither of those two oplines? If an exception is thrown?

This comment has been minimized.

@dstogov

dstogov Feb 13, 2019

Author Contributor

Comparison opcodes may bypass smart-branch logic on slow patch and transfer control to the folloeing JMPZ/JMPNZ instruction

<?php
$a = 5;
$b = "10";
if ($a == $b) {
        var_dump(1);
} else {
        var_dump(2);
}

dstogov added some commits Feb 13, 2019

Merge branch 'master' into jit-dynasm
* master:
  Update NEWS and UPGRADING [ci skip]
  Change the way timer queue timer is deleted
  Add test socket_setopt() basic functionality
  Fix test
  Fixed bug #76430
  Implement mb_str_split()
  Use TSRM macros
  More accurate handling of global registers (allow VM with single global register)
  Correct section number in UPGRADING
  Fix FFI test on Windows
  Fixed bug #75546
  typo
  Fixed bug #77608
  security fix - by default 'local infile' is disabled: - set default for mysqli.allow_local_infile=0 - explicitly disable PDO::MYSQL_ATTR_LOCAL_INFILE in case of lack of driver options - add getAttribute support for PDO::MYSQL_ATTR_LOCAL_INFILE - update existing tests where needed - add new tests [checking default value and setting on] the 'local infile' in ext/mysqli and ext/pdo_mysql
  Use pkg-config for ICU, as the old icu-config has been deprecated
@dstogov

This comment has been minimized.

Copy link
Contributor Author

dstogov commented Feb 13, 2019

@nikic I've added comments according to your last review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment