-
Notifications
You must be signed in to change notification settings - Fork 1
Pattern matching 3 #165
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
base: master
Are you sure you want to change the base?
Pattern matching 3 #165
Conversation
f4740b7 to
0f2d513
Compare
|
So, the current impl essentially:
This certainly works. It will not match the performance of manually doing it. I.e. It would compile and and or operations right away the same than you do compile a short-circuiting and-operation, and know that optimizer knows how to handle that pattern. And then you can remove the true/false branch handling for patterns without bindings. On the topic of temporaries, I think this code is as good as it gets, with still using proper TMPs. The only thing I see in zend_pm_compile_container is that you could skip using the TMP (and doing copies) if expr_node is already CV. And skip the copy on the last iteration. (And yeah, you know, that thing which may need liveness or optimizer adaptions...) To actually unlock any benefits here, we'll have to stop copying TMPs around. I.e. you keep a list of fake temporaries (note that there is no need to free immediately - you free them immediately if you have no further need for them, but otherwise you just free them at the end), and just put our current value into the temporary for the current nesting + number of bindings depth - and on failure, you jump right to the nth ZEND_FREE, according to the current depth. And then, on success, you just do the assignments and free the rest and jump over the false branch. |
|
My idea for avoiding the final true/false branch without bindings (that so far doesn't work) was a bit different. I tried passing The problem however is that smart branchable comparisons dodge assigning to the temporary when followed by a jump. Hence the value ends-up not actually being assigned to... Other than that, this would have been a pretty clean solution. I'll see whether your solution is applicable. I believe the downside is that every jump would then assign to result, or we'd need to figure out which jumps can omit it.. |
I'm pretty sure there already exist current violations, but because it's bool, it's fine. (I.e. live range calculation explicitly ignores ZEND_JMPZ_EX, ZEND_JMPNZ_EX, ZEND_BOOL and ZEND_BOOL_NOT.)
Yep, realized the same thing - I actually looked that up when writing my last comment :-D |
c6e9505 to
8a4a6f0
Compare
Topic of discussion for the community.
8a4a6f0 to
c7024e4
Compare
No description provided.