Skip to content

Conversation

@iluuu1994
Copy link
Owner

No description provided.

@bwoebi
Copy link

bwoebi commented Nov 21, 2025

So, the current impl essentially:

  • put nulls into tmps for bindings
  • execute the pattern - which jumps to false on failure
  • assign tmps into the CVs
  • assign boolean true as result
  • jump over false branch
  • false branch: free all tmps and assign boolean false as result

This certainly works. It will not match the performance of manually doing it. I.e. if ($foo is Foo { a: 42, b: 43 }) will be slower than if ($foo instanceof Foo && $foo->a == 42 && $foo->b == 42).
Because the latter does not have to assign true or false to a result, after optimization.

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.
I.e. take inspiration from zend_compile_short_circuiting() - i.e. directly emit JMP(N)Z instructions in zend_pm_compile_or/and.
The way it's currently done heavily optimizes for and, and makes the or handling quite awkward.

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.
I know this is a bit condensed. Would have to implement it to be clearer :-X
But as said, there's no need to do this in the first iteration we'll vote on. We can always improve after initial merge.

@iluuu1994
Copy link
Owner Author

iluuu1994 commented Nov 21, 2025

My idea for avoiding the final true/false branch without bindings (that so far doesn't work) was a bit different. I tried passing result into every compile function, which is intended to hold any temporary comparison result (the same var for all of them). Logically, at the end of the match, it should contain the final true/false value to indicate whether the pattern matched. This does technically violate operand consumption rules (as we potentially branch on the var and then use it as a result after the pattern match), which shouldn't be a problem because it's not a refcounted value.

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..

@bwoebi
Copy link

bwoebi commented Nov 21, 2025

technically violate operand consumption rules

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.)

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

Yep, realized the same thing - I actually looked that up when writing my last comment :-D
But ultimately, when you have ZEND_IS_IDENTICAL+ZEND_BOOL+ZEND_JMPNZ, optimizer will optimize it into a smart branch. Which should be fine then. The only annoyance is the FREE's, but if there are no bindings, there's also no FREE to be done - in success case, that is. In failure case it depends.

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.

3 participants