Skip to content

Optimize special functions with help of JMP_FRAMELESS #18592

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions Zend/tests/bug74164.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ call_user_func(function (array &$ref) {var_dump("xxx");}, 'not_an_array_variable
--EXPECTF--
Fatal error: Uncaught Exception: {closure:%s:%d}(): Argument #1 ($ref) must be passed by reference, value given in %s:%d
Stack trace:
#0 [internal function]: {closure:%s:%d}(2, '%s', '%s', 9)
#1 %sbug74164.php(%d): call_user_func(%s)
#2 {main}
#0 %s(%d): {closure:%s:%d}(2, '%s', '%s', 9)
#1 {main}
thrown in %sbug74164.php on line %d
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,9 @@ namespace Foo;
var_dump(call_user_func('strlen', false));

?>
--EXPECT--
int(0)
--EXPECTF--
Fatal error: Uncaught TypeError: strlen(): Argument #1 ($string) must be of type string, false given in %s:%d
Stack trace:
#0 %s(%d): strlen(false)
#1 {main}
thrown in %s on line %d
105 changes: 104 additions & 1 deletion Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ static zend_op *zend_delayed_compile_var(znode *result, zend_ast *ast, uint32_t
static void zend_compile_expr(znode *result, zend_ast *ast);
static void zend_compile_stmt(zend_ast *ast);
static void zend_compile_assign(znode *result, zend_ast *ast);
static zend_result zend_try_compile_special_func(znode *result, zend_string *lcname, zend_ast_list *args, zend_function *fbc, uint32_t type);
static bool zend_is_special_func(const zend_string *name);

#ifdef ZEND_CHECK_STACK_LIMIT
zend_never_inline static void zend_stack_limit_error(void)
Expand Down Expand Up @@ -4709,6 +4711,7 @@ static uint32_t zend_compile_frameless_icall(znode *result, zend_ast_list *args,
static void zend_compile_ns_call(znode *result, znode *name_node, zend_ast *args_ast, uint32_t lineno, uint32_t type) /* {{{ */
{
int name_constants = zend_add_ns_func_name_literal(Z_STR(name_node->u.constant));
bool special_func_handler = false;

/* Find frameless function with same name. */
zend_function *frameless_function = NULL;
Expand All @@ -4725,7 +4728,13 @@ static void zend_compile_ns_call(znode *result, znode *name_node, zend_ast *args
const zend_frameless_function_info *frameless_function_info = NULL;
if (frameless_function) {
frameless_function_info = find_frameless_function_info(zend_ast_get_list(args_ast), frameless_function, type);
if (frameless_function_info) {

if (!frameless_function_info) {
/* Check for dedicated special function. */
special_func_handler = zend_is_special_func(frameless_function->common.function_name);
}

if (frameless_function_info || special_func_handler) {
CG(context).in_jmp_frameless_branch = true;
znode op1;
op1.op_type = IS_CONST;
Expand Down Expand Up @@ -4759,6 +4768,44 @@ static void zend_compile_ns_call(znode *result, znode *name_node, zend_ast *args
SET_NODE(flf_icall->result, result);
zend_update_jump_target_to_next(jmp_end_opnum);

CG(context).in_jmp_frameless_branch = false;
} else if (special_func_handler) {
CG(zend_lineno) = lineno;

uint32_t jmp_end_opnum = zend_emit_jump(0);
uint32_t jmp_fl_target = get_next_op_number();

znode tmp_result;
if (zend_try_compile_special_func(&tmp_result, frameless_function->common.function_name, zend_ast_get_list(args_ast), frameless_function, type) != SUCCESS) {
zend_op *ops = CG(active_op_array)->opcodes;

/* Undo work */
zend_op *jmp_fl = &ops[jmp_fl_opnum];
jmp_fl->opcode = ZEND_NOP;
SET_UNUSED(jmp_fl->op1);
zend_op *jmp_end = &ops[jmp_end_opnum];
jmp_end->opcode = ZEND_NOP;
SET_UNUSED(jmp_end->op1);
} else {
uint32_t result_target = get_next_op_number() - 1;

zend_op *jmp_fl = &CG(active_op_array)->opcodes[jmp_fl_opnum];
jmp_fl->op2.opline_num = jmp_fl_target;
jmp_fl->extended_value = zend_alloc_cache_slot();

if (tmp_result.op_type == IS_CONST || 1 /* TODO ? */) {
zend_op *qm_assign = zend_emit_op(NULL, ZEND_QM_ASSIGN, &tmp_result, NULL);
SET_NODE(qm_assign->result, result);
} else {
// TODO: we're left with a useless tmp?
zend_op *res_op = &CG(active_op_array)->opcodes[result_target];
res_op->result_type = result->op_type;
res_op->result = result->u.op;
}

zend_update_jump_target_to_next(jmp_end_opnum);
}

CG(context).in_jmp_frameless_branch = false;
}
}
Expand Down Expand Up @@ -4936,6 +4983,62 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
return SUCCESS;
}

typedef struct zend_special_func {
const char *name;
size_t name_len;
// zend_result (*handler)(znode *, zend_ast_list *);
} zend_special_func;

static const zend_special_func zend_special_func_compile_handlers[] = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a comment with the definition of the explicitly listed functions, ie. what they need to meet and if not all are listed, why only smaller subset is listed.

{ZEND_STRL("strlen")},
{ZEND_STRL("is_null")},
{ZEND_STRL("is_bool")},
{ZEND_STRL("is_long")},
{ZEND_STRL("is_int")},
{ZEND_STRL("is_integer")},
{ZEND_STRL("is_float")},
{ZEND_STRL("is_double")},
{ZEND_STRL("is_string")},
{ZEND_STRL("is_array")},
{ZEND_STRL("is_object")},
{ZEND_STRL("is_resource")},
{ZEND_STRL("is_scalar")},
{ZEND_STRL("boolval")},
{ZEND_STRL("intval")},
{ZEND_STRL("floatval")},
{ZEND_STRL("doubleval")},
{ZEND_STRL("strval")},
{ZEND_STRL("defined")},
{ZEND_STRL("chr")},
{ZEND_STRL("ord")},
{ZEND_STRL("call_user_func_array")},
{ZEND_STRL("call_user_func")},
{ZEND_STRL("in_array")},
{ZEND_STRL("count")},
{ZEND_STRL("sizeof")},
{ZEND_STRL("get_class")},
{ZEND_STRL("get_called_class")},
{ZEND_STRL("gettype")},
{ZEND_STRL("func_num_args")},
{ZEND_STRL("func_get_args")},
{ZEND_STRL("array_slice")},
{ZEND_STRL("array_key_exists")},
{ZEND_STRL("sprintf")},
{NULL, 0},
};

static bool zend_is_special_func(const zend_string *name)
{
const zend_special_func *entry = &zend_special_func_compile_handlers[0];
for (; entry->name; entry++) {
if (zend_string_equals_cstr(name, entry->name, entry->name_len)) {
return true;
}
}
return false;
}

// TODO: maybe this can migrate to use 'zend_special_func_compile_handlers'
static zend_result zend_try_compile_special_func_ex(znode *result, zend_string *lcname, zend_ast_list *args, zend_function *fbc, uint32_t type) /* {{{ */
{
if (zend_string_equals_literal(lcname, "strlen")) {
Expand Down
27 changes: 6 additions & 21 deletions ext/reflection/tests/ReflectionFiber_notrace_2.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@ var_dump($reflection->getTrace());
--EXPECTF--
string(%d) "%sReflectionFiber_notrace_2.php"
int(5)
array(3) {
array(2) {
[0]=>
array(4) {
array(6) {
["file"]=>
string(%d) "%s"
["line"]=>
int(5)
["function"]=>
string(7) "suspend"
["class"]=>
Expand All @@ -32,25 +36,6 @@ array(3) {
}
}
[1]=>
array(4) {
["file"]=>
string(%d) "%sReflectionFiber_notrace_2.php"
["line"]=>
int(5)
["function"]=>
string(14) "call_user_func"
["args"]=>
array(1) {
[0]=>
array(2) {
[0]=>
string(5) "Fiber"
[1]=>
string(7) "suspend"
}
}
}
[2]=>
array(2) {
["function"]=>
string(%d) "{closure:%s:%d}"
Expand Down
Loading