-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Avoid branches in ZEND_BOOL/ZEND_BOOL_NOT #4912
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
Conversation
gcc 5/9 will add 2 (IS_FALSE) to the boolean result of 1/2 to compute the value to store for the ZVAL_BOOL call. Additionally, this puts all of the reads of Z_TYPE_INFO_P before writes to memory (ZVAL_BOOL), so that php doesn't have to load from memory into a register again. (gcc 5 would use branches for evaluating ZVAL_BOOL if ZVAL_BOOL was executed before comparing against IS_UNDEF) A contrived example heavily using ZVAL_BOOL with poor branch prediction ```php // With gcc 9, cpufreq 2.5GZ // before: 0.305 seconds with 1000, 10000 // after 0.286 seconds with 1000, 10000 function loop_bool_not(int $a, int $b) { srand($a); $values = []; for ($i = 0; $i < $a; $i++) { $values[] = (bool)(\rand() & 1); } $count = []; for ($i = 0; $i < $b; $i++) { foreach ($values as $v) { $count[!$v]++; } } return $count; } ``` The true then falsey check was first added in e6180eb
@TysonAndre I don't think this micro-optimization makes a lot of sense. It may improve some special cases in hot loops, but also may lead to degradation, if CPU can't predict two conditional branches before the one that you optimize. Reloading type from L1 cache is less expensive, than complication of prologue/epilogue. Anyway, the effect it too small even on synthetic test. |
If nobody thinks this refactoring makes sense, there's still the below refactoring in BOOL_NOT and BOOL I could make. This avoids the second read from memory into the register of Z_TYPE_INFO_P, because the compiler can't tell that ZVAL_TRUE isn't writing to the same byte as the return value's type. --- a/Zend/zend_vm_def.h
+++ b/Zend/zend_vm_def.h
@@ -940,8 +940,9 @@ ZEND_VM_COLD_CONST_HANDLER(14, ZEND_BOOL_NOT, CONST|TMPVAR|CV, ANY)
if (Z_TYPE_INFO_P(val) == IS_TRUE) {
ZVAL_FALSE(EX_VAR(opline->result.var));
} else if (EXPECTED(Z_TYPE_INFO_P(val) <= IS_TRUE)) {
+ const uint32_t val_type = Z_TYPE_INFO_P(val);
ZVAL_TRUE(EX_VAR(opline->result.var));
- if (OP1_TYPE == IS_CV && UNEXPECTED(Z_TYPE_INFO_P(val) == IS_UNDEF)) {
+ if (OP1_TYPE == IS_CV && UNEXPECTED(val_type == IS_UNDEF)) { |
The VM optimization become an endless fight with C compilers long time ago :) |
And `$x = !$x` Noticed while working on phpGH-4912 The included test would not emit undefined variable errors in php 8.0 with opcache enabled. The command used: ``` php -d zend_extension=opcache.so --no-php-ini -d error_reporting=E_ALL \ -d opcache.file_cache= -d opcache.enable_cli=1 test.php ```
And `$x = !$x` Noticed while working on phpGH-4912 The included test would not emit undefined variable errors in php 8.0 with opcache enabled. The command used: ``` php -d zend_extension=opcache.so --no-php-ini -d error_reporting=E_ALL \ -d opcache.file_cache= -d opcache.enable_cli=1 test.php ```
And `$x = !$x` Noticed while working on phpGH-4912 The included test would not emit undefined variable errors in php 8.0 with opcache enabled. The command used: ``` php -d zend_extension=opcache.so --no-php-ini -d error_reporting=E_ALL \ -d opcache.file_cache= -d opcache.enable_cli=1 test.php ```
And `$x = !$x` Noticed while working on phpGH-4912 The included test would not emit undefined variable errors in php 8.0 with opcache enabled. The command used: ``` php -d zend_extension=opcache.so --no-php-ini -d error_reporting=E_ALL \ -d opcache.file_cache= -d opcache.enable_cli=1 test.php ```
Closing in favor of #4914 - this gets called far less often than I thought. |
And `$x = !$x` Noticed while working on GH-4912 The included test would not emit undefined variable errors in php 8.0 with opcache enabled. The command used: ``` php -d zend_extension=opcache.so --no-php-ini -d error_reporting=E_ALL \ -d opcache.file_cache= -d opcache.enable_cli=1 test.php ```
gcc 5/9 will add 2 (IS_FALSE) to the boolean result of 1/0 to compute
the value to store for the ZVAL_BOOL call.
Additionally, this puts all of the reads of Z_TYPE_INFO_P before writes
to memory (ZVAL_BOOL),
so that php doesn't have to load from memory into a register again.
(gcc 5 would use branches for evaluating ZVAL_BOOL if ZVAL_BOOL was
executed before comparing against IS_UNDEF)
A contrived example heavily using ZVAL_BOOL with poor branch prediction
The true then falsey check was first added in
e6180eb
Miscellaneous thoughts:
return !$int
slightly due to one less comparison (SCCP and opcache help remove this opcode for obvious booleans such as(bool)is_string($x)
)Before:
After: