Skip to content

Perf: undefined-variable guards are emitted for parameters and definitely-assigned locals through module-global flags (lib/JIT/UndefinedVariableHelper.php, lib/JIT/ScopeVariableAssignedFlags.php) #36190

Description

@PurHur

Category

Regression: · generated-code performance / semantics · child of #36188

Problem

fibo_r(int $n) compiled on master 4eed6a2785 does this at entry and before every read of the parameter $n:

movabs $0x16890b8,%r15        ; phpc_scope_var_init_7ce6360daf283396 (module global, i8)
movb   $0x1,(%r15)            ; "assigned" at entry
...
cmpb   $0x0,(%r15)            ; before each read of $n
jne    ok
call   __compiler_trigger_error   ; "Undefined variable $n" cold path

ScopeVariableAssignedFlags::ensureFlag() (lib/JIT/ScopeVariableAssignedFlags.php:29-37) creates one module-level global per (function, variable) and UndefinedVariableHelper::guardBeforeRuntimeRead() (lib/JIT/UndefinedVariableHelper.php:64-) emits the test at every runtime read. resolveTrackableName() skips only $this, superglobals and include bindings — parameters are guarded, although a parameter can never be undefined.

Two consequences:

  1. Cost: a global RMW + branch per variable read in every hot loop and every recursive call; combined with the boxing issue it is why fibo_r is ~400 instructions.
  2. Semantics: the flag is a process-wide global, not per activation. A recursive or re-entrant call that assigns $x marks the flag for all frames of that function, so a later frame that reads $x before assigning it gets no warning (Zend warns). Conversely unset($x) in one frame must not affect another. The guard can be both slower and wrong.

php-src reference

  • Zend/zend_execute.cZEND_HANDLE_EXCEPTION/zval_undefined_cv: the check is on the CV slot in the current frame (Z_TYPE_P(cv) == IS_UNDEF), never on global state; compiled code specialises IS_CV reads whose definedness is proven by the optimizer (zend_ssa "definitely assigned").

PHP implementation target

  • Compute definite assignment on the php-cfg SSA form before lowering: parameters, $this, and every variable assigned on all paths to a read need no guard. Only "maybe-undefined" reads keep one.
  • For the remaining maybe-undefined reads use a per-frame alloca i1 (or the value slot's own "undef" type tag, as Zend does) instead of a module global; reset semantics follow the frame.
  • Keep the dynamic-scope fallback (extract, compact, $$name, include binding, global) on the existing path.
  • Files: lib/JIT/UndefinedVariableHelper.php, lib/JIT/ScopeVariableAssignedFlags.php, the CV read lowering in lib/JIT.php (Undefined variable E_WARNING site ~L10689), lib/JIT/ScopeBuiltinEmitHelper.php:347.

Repro

./script/docker-exec.sh -- bash -lc 'source script/php-env.sh \
  && php bin/compile.php -o build/fibo30 "benchmarks/fibo(30).php" \
  && nm build/fibo30 | grep -c phpc_scope_var_init_ \
  && objdump -d --disassemble=fibo_r build/fibo30 | grep -c trigger_error'

Expected after fix: 0 flags for fibo_r, 0 __compiler_trigger_error sites in it.

Semantics probe (must warn twice under Zend and AOT):

<?php function f(int $d) { if ($d > 0) { $x = 1; f($d - 1); } echo isset($x) ? "set" : "unset", " ", @$x, "\n"; } f(1);

Done when

  • No phpc_scope_var_init_* global is emitted for parameters or definitely-assigned locals; per-frame state for the rest
  • Recursion probe above matches Zend byte-for-byte (script/differential-sweep.sh --dir case added to test/differential/cases/)
  • test/compliance/cases/language/*undefined* and *undef* failing-name set unchanged vs master
  • fibo_r instruction count drops (measured in the sibling boxing issue)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    IMPORTANTCritical patharea:compilerCompiler / CFG / JITbugSomething isn't workingimplementation-readySpec complete: repro, php-src ref, done-when — safe for workers to claimphase-3:aotPhase 3 – AOT deployment

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions