Transparently introduce type-specialized opcode handlers. #1825
Conversation
This affects only PHP VM, and doesn't change anything else.
|
I'm probably missing something obvious here, but how does this interact with the file cache? Won't it reassign the unoptimized handler? |
|
You are right [☹] From: Nikita Popov notifications@github.com I'm probably missing something obvious here, but how does this interact with the file cache? Won't it reassign the unoptimized handler? You are receiving this because you authored the thread. |
| ZEND_VM_NEXT_OPCODE(); | ||
| } | ||
|
|
||
| ZEND_VM_TYPE_SPEC_HANDLER(ZEND_POST_INC, (res_info == MAY_BE_LONG && op1_info == MAY_BE_LONG), ZEND_POST_INC_LONG_NO_OVERFLOW, TMPVARCV, ANY) |
There was a problem hiding this comment.
I don't think this is correct. If op1 == ZEND_LONG_MAX then result would be long, but the op1 increment would overflow into double. We should be (somehow) checking the type of op1_def here instead.
There was a problem hiding this comment.
I'm guessing that ZEND_POST_INC_LONG_NO_OVERFLOW here means this will never be used where it could overflow.
There was a problem hiding this comment.
Right. For PRE/POST_INC/DEC optimizer passes op1_def into result_info
|
Introducing special versions of opcodes that can only be used when Optimizer is enabled irks me somehow. Isn't Optimizer still not turned on by default and only built with OPcache, as a shared library? |
| end = opline + op_array->last; | ||
| while (opline < end) { | ||
| zend_vm_set_opcode_handler_ex(opline, | ||
| opline->op1_type == IS_UNDEF ? 0 : (OP1_INFO() & (MAY_BE_UNDEF|MAY_BE_ANY|MAY_BE_REF|MAY_BE_ARRAY_OF_ANY|MAY_BE_ARRAY_KEY_ANY)), |
There was a problem hiding this comment.
Should be IS_UNUSED (and also a couple of times below).
|
Now optimiser is independent from opcache and may be moved into Zend, but it won't make a lot of sense to use it without opcache anyway. From: Andrea Faulds notifications@github.com Introducing special versions of opcodes that can only be used when Optimizer is enabled irks me somehow. Isn't Optimizer still not turned on by default and only built with OPcache, as a shared library? You are receiving this because you authored the thread. |
|
Yeah, it's true that using Optimizer without OPcache is probably a waste of time, given how it would just slow down your requests. Anyway, I think moving Optimizer into Zend would make me happier, even if it's mostly just cosmetic. Perhaps we could move OPcache itself at some point, too. |
|
Merged into master |
This affects only PHP VM, and doesn't change anything else.
Followup to #1824.