Repository navigation
Conversation
| if (use_block != def_block) { | ||
| bb = &ssa->cfg.blocks[use_block]; | ||
| bb = &ssa->cfg.blocks[p->block]; | ||
| if (bb->flags & (ZEND_BB_ENTRY|ZEND_BB_RECV_ENTRY)) { |
There was a problem hiding this comment.
I'm confused. So the block after the DO_UCALL opcode is an entry block, reachable via a fake edge successor? And so the frontend has to make sure values that can go across those fake edges into an entry are not in a register? That's wild.
I wonder if this is the best approach, it's also very difficult to understand imo
There was a problem hiding this comment.
Yes. Under ZEND_CFG_STACKLESS the block after DO_UCALL/DO_FCALL/DO_FCALL_BY_NAME (and include/yield) is an ENTRY: for user calls the JIT code tail-calls into the VM and the callee comes back through it, so no register survives that edge. zend_jit_may_be_in_reg() has rejected such values since caf102d, and this only fixes two cases its walk missed.
There was a problem hiding this comment.
Is this the cleanest way to fix it? At least some comments are necessary since it's difficult to follow
There was a problem hiding this comment.
Restructured the Phi-use loop and commented each case. It's the narrowest fix I found: the walk already handles every other use, and this extends it to the two Phi cases it skipped.
81d3c93 to
b207515
Compare
ndossche
left a comment
ndossche
left a comment
There was a problem hiding this comment.
Now a value used by a phi in an entry block can never live in a register, even if that doesn't come from the VM in an incoming edge. Shouldn't using ZREG_STORE be more precise?
A Phi in an ENTRY block now always moves through memory: its sources are stored and the Phi is reloaded, because the VM may enter that block without the caller's registers. The ENTRY walk in zend_jit_may_be_in_reg() also follows the incoming edges of a loop-carried Phi use in the definition block, except for a zero-length Pi interval. DO_UCALL, DO_FCALL, and DO_FCALL_BY_NAME reproduce the bug; yield and include do not. Fixes phpGH-25164
b207515 to
9f92047
Compare
|
Indeed. Switched to ZREG_STORE: a Phi in an ENTRY block now always forces a move, so its sources are stored and the Phi reloads, instead of those sources being excluded from registers. |
Function JIT kept values in registers across user function calls. Phis in ENTRY blocks now always store their sources and reload, and the ENTRY walk in
zend_jit_may_be_in_reg()now follows loop-carried Phi uses in the definition block, keeping zero-length Pi intervals in registers. #18392 addresses a separate EX(call) cache issue.Fixes GH-25164