[PATCH v21 0/6] MR10515: Draft: vbscript: Bind local variables, class properties, and loop variables at compile time.
Resolve identifiers to direct indices during compilation instead of doing runtime `wcsicmp` string scans in `lookup_identifier()`. This adds new opcodes that access local variables, function arguments, class properties, and loop variables by index, eliminating the linear string comparison overhead for the most common cases. **New opcodes:** - `OP_local` / `OP_assign_local` / `OP_set_local` - read/write local vars and args by index - `OP_local_prop` / `OP_assign_local_prop` / `OP_set_local_prop` - read/write class properties by index - `OP_step_local` / `OP_incc_local` - For-loop step/increment with indexed counter - `OP_enumnext_local` - For-Each iteration with indexed loop variable This mirrors jscript's `OP_local` / `bind_local()` optimization (`dlls/jscript/compile.c`) and addresses the FIXME in `lookup_identifier()` noting that class property access should be bound at compile time. Names that cannot be resolved at compile time (globals in Execute/ExecuteGlobal code, dynamic vars, host objects) continue to use the existing string-based path. The last commit also adds a fast path for SAFEARRAY iteration that bypasses the `IEnumVARIANT` COM vtable dispatch and copies elements directly from the array data, eliminating one intermediate `VariantCopy` per iteration. ### Local variable / argument / class property access (1M iterations, 10 reads per iteration) | Benchmark | Windows (VM) | Wine master | Wine (7 commits) | **Speedup** | vs Windows | |-----------|-------------|-------------|-------------------|---------|------------| | Local vars | 156 ms | 413 ms | 234 ms | **1.8x** | 1.5x slower | | Arguments | 171 ms | 357 ms | 231 ms | **1.5x** | 1.4x slower | | Class props | 265 ms | 393 ms | 226 ms | **1.7x** | 0.9x (faster!) | ### For-loop variants (10M iterations) | Benchmark | Windows (VM) | Wine master | Wine (7 commits) | **Speedup** | vs Windows | |-----------|-------------|-------------|-------------------|---------|------------| | Empty For | 117 ms | 370 ms | 270 ms | **1.4x** | 2.3x slower | | For + assign | 203 ms | 613 ms | 430 ms | **1.4x** | 2.1x slower | | Do-While manual | 437 ms | 646 ms | 513 ms | **1.3x** | 1.2x slower | | For Step 2 | 62 ms | 370 ms | 271 ms | **1.4x** | 4.4x slower | | Nested For | 125 ms | 387 ms | 268 ms | **1.4x** | 2.1x slower | | For local Sub | 109 ms | 294 ms | 251 ms | **1.2x** | 2.3x slower | | For R8 counter | 203 ms | 2,812 ms | 2,529 ms | **1.1x** | 12x slower | | For-Each array | 7 ms | 286 ms | 62 ms | **4.6x** | 8.9x slower | The remaining 4-13x gap vs Windows in For-loops is from `VarAdd`/`VarCmp` going through full `VariantChangeTypeEx` machinery (including locale alloc/free per call). A separate MR ([!10528](https://gitlab.winehq.org/wine/wine/-/merge_requests/10529)) adds I2/I4 fast paths in oleaut32 that bring integer loops within 1.2-1.9x of Windows when combined with this branch. -- v21: https://gitlab.winehq.org/wine/wine/-/merge_requests/10515
From: Francis De Brabandere <francisdb@gmail.com> Resolve Dim variables and function arguments to direct indices during compilation, emitting OP_local instead of OP_ident. This eliminates runtime wcsicmp scans in lookup_identifier() for the most common case. This mirrors jscript's OP_local optimization (dlls/jscript/compile.c: bind_local / emit_identifier). --- dlls/vbscript/compile.c | 41 +++++++++++++++++++++++++++++++++++----- dlls/vbscript/interp.c | 21 ++++++++++++++++++++ dlls/vbscript/vbscript.h | 1 + 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 54314372a13..713834ee5e6 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -456,6 +456,34 @@ static BOOL lookup_dim_decls(compile_ctx_t *ctx, const WCHAR *name) return FALSE; } +/* Resolve an identifier to a local variable or argument index at compile time. + * Returns TRUE if bound, with *ret set to: non-negative = dim var index, negative = arg index (-1 = arg 0, etc.). + * This mirrors jscript's bind_local() / local_off() convention. */ +static BOOL bind_local(compile_ctx_t *ctx, const WCHAR *name, int *ret) +{ + dim_decl_t *dim_decl; + unsigned i; + + if(ctx->func->type == FUNC_GLOBAL) + return FALSE; + + for(dim_decl = ctx->dim_decls, i = 0; dim_decl; dim_decl = dim_decl->next, i++) { + if(!vbs_wcsicmp(dim_decl->name, name)) { + *ret = i; + return TRUE; + } + } + + for(i = 0; i < ctx->func->arg_cnt; i++) { + if(!vbs_wcsicmp(ctx->func->args[i].name, name)) { + *ret = -(int)i - 1; + return TRUE; + } + } + + return FALSE; +} + static HRESULT compile_args(compile_ctx_t *ctx, expression_t *args, unsigned *ret) { unsigned arg_cnt = 0; @@ -506,15 +534,18 @@ static HRESULT compile_member_call_expression(compile_ctx_t *ctx, member_express static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t *expr) { expression_t *const_expr; + int local_ref; if (expr->obj_expr) /* FIXME: we should probably have a dedicated opcode as well */ return compile_member_call_expression(ctx, expr, 0, TRUE); - if (!lookup_dim_decls(ctx, expr->identifier) && !lookup_args_name(ctx, expr->identifier)) { - const_expr = lookup_const_decls(ctx, expr->identifier, TRUE); - if(const_expr) - return compile_expression(ctx, const_expr); - } + if(bind_local(ctx, expr->identifier, &local_ref)) + return push_instr_int(ctx, OP_local, local_ref); + + const_expr = lookup_const_decls(ctx, expr->identifier, TRUE); + if(const_expr) + return compile_expression(ctx, const_expr); + return push_instr_bstr(ctx, OP_ident, expr->identifier); } diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index ba530fca38c..8804b13646b 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -873,6 +873,27 @@ static HRESULT interp_mcallv(exec_ctx_t *ctx) return do_mcall(ctx, NULL); } +static HRESULT interp_local(exec_ctx_t *ctx) +{ + const int arg = ctx->instr->arg1.lng; + VARIANT *v; + VARIANT r; + + if(arg < 0) + v = ctx->args - arg - 1; + else + v = ctx->vars + arg; + + TRACE("%s\n", debugstr_variant(v)); + + if(V_VT(v) == (VT_BYREF|VT_VARIANT)) + v = V_VARIANTREF(v); + + V_VT(&r) = VT_BYREF|VT_VARIANT; + V_BYREF(&r) = v; + return stack_push(ctx, &r); +} + static HRESULT interp_ident(exec_ctx_t *ctx) { BSTR identifier = ctx->instr->arg1.bstr; diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 19c9260a342..353aab3a9e3 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -270,6 +270,7 @@ typedef enum { X(incc, 1, ARG_BSTR, 0) \ X(int, 1, ARG_INT, 0) \ X(is, 1, 0, 0) \ + X(local, 1, ARG_INT, 0) \ X(jmp, 0, ARG_ADDR, 0) \ X(jmp_false, 0, ARG_ADDR, 0) \ X(jmp_true, 0, ARG_ADDR, 0) \ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10515
From: Francis De Brabandere <francisdb@gmail.com> Extract the REF_VAR body of assign_ident into a new assign_local_var helper so that future compile-time-bound local assignments can reuse it instead of duplicating the logic. No functional change. --- dlls/vbscript/interp.c | 86 +++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index 8804b13646b..af62c02680d 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -942,61 +942,63 @@ static HRESULT assign_value(exec_ctx_t *ctx, VARIANT *dst, VARIANT *src, WORD fl return S_OK; } -static HRESULT assign_ident(exec_ctx_t *ctx, BSTR name, WORD flags, DISPPARAMS *dp) +static HRESULT assign_local_var(exec_ctx_t *ctx, VARIANT *v, WORD flags, DISPPARAMS *dp) { - ref_t ref; HRESULT hres; - hres = lookup_identifier(ctx, name, VBDISP_LET, &ref); - if(FAILED(hres)) - return hres; + if(V_VT(v) == (VT_VARIANT|VT_BYREF)) + v = V_VARIANTREF(v); - switch(ref.type) { - case REF_VAR: { - VARIANT *v = ref.u.v; + if(arg_cnt(dp)) { + SAFEARRAY *array; - if(V_VT(v) == (VT_VARIANT|VT_BYREF)) - v = V_VARIANTREF(v); + if(V_VT(v) == VT_DISPATCH) + return disp_propput(ctx->script, V_DISPATCH(v), DISPID_VALUE, flags, dp); - if(arg_cnt(dp)) { - SAFEARRAY *array; + if(!(V_VT(v) & VT_ARRAY)) + return DISP_E_TYPEMISMATCH; - if(V_VT(v) == VT_DISPATCH) { - hres = disp_propput(ctx->script, V_DISPATCH(v), DISPID_VALUE, flags, dp); - break; - } + switch(V_VT(v)) { + case VT_ARRAY|VT_BYREF|VT_VARIANT: + array = *V_ARRAYREF(v); + break; + case VT_ARRAY|VT_VARIANT: + array = V_ARRAY(v); + break; + default: + FIXME("Unsupported array type %x\n", V_VT(v)); + return E_NOTIMPL; + } - if(!(V_VT(v) & VT_ARRAY)) - return DISP_E_TYPEMISMATCH; + if(!array) { + WARN("null array\n"); + return MAKE_VBSERROR(VBSE_OUT_OF_BOUNDS); + } - switch(V_VT(v)) { - case VT_ARRAY|VT_BYREF|VT_VARIANT: - array = *V_ARRAYREF(v); - break; - case VT_ARRAY|VT_VARIANT: - array = V_ARRAY(v); - break; - default: - FIXME("Unsupported array type %x\n", V_VT(v)); - return E_NOTIMPL; - } + hres = array_access(array, dp, &v); + if(FAILED(hres)) + return hres; + }else if(V_VT(v) == (VT_ARRAY|VT_BYREF|VT_VARIANT)) { + FIXME("non-array assign\n"); + return E_NOTIMPL; + } - if(!array) { - WARN("null array\n"); - return MAKE_VBSERROR(VBSE_OUT_OF_BOUNDS); - } + return assign_value(ctx, v, dp->rgvarg, flags); +} - hres = array_access(array, dp, &v); - if(FAILED(hres)) - return hres; - }else if(V_VT(v) == (VT_ARRAY|VT_BYREF|VT_VARIANT)) { - FIXME("non-array assign\n"); - return E_NOTIMPL; - } +static HRESULT assign_ident(exec_ctx_t *ctx, BSTR name, WORD flags, DISPPARAMS *dp) +{ + ref_t ref; + HRESULT hres; + + hres = lookup_identifier(ctx, name, VBDISP_LET, &ref); + if(FAILED(hres)) + return hres; - hres = assign_value(ctx, v, dp->rgvarg, flags); + switch(ref.type) { + case REF_VAR: + hres = assign_local_var(ctx, ref.u.v, flags, dp); break; - } case REF_DISP: hres = disp_propput(ctx->script, ref.u.d.disp, ref.u.d.id, flags, dp); break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10515
From: Francis De Brabandere <francisdb@gmail.com> Emit OP_assign_local and OP_set_local instead of OP_assign_ident and OP_set_ident when the assignment target is a known local variable or function argument. This eliminates runtime lookup_identifier() calls for the most common assignment pattern. This mirrors jscript's OP_local optimization for identifier writes. --- dlls/vbscript/compile.c | 28 ++++++++++++++++++++++++ dlls/vbscript/interp.c | 46 ++++++++++++++++++++++++++++++++++++++++ dlls/vbscript/vbscript.h | 2 ++ 3 files changed, 76 insertions(+) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 713834ee5e6..890d024a276 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -192,6 +192,19 @@ static HRESULT push_instr_uint(compile_ctx_t *ctx, vbsop_t op, unsigned arg) return S_OK; } +static HRESULT push_instr_int_uint(compile_ctx_t *ctx, vbsop_t op, LONG arg1, unsigned arg2) +{ + unsigned ret; + + ret = push_instr(ctx, op); + if(!ret) + return E_OUTOFMEMORY; + + instr_ptr(ctx, ret)->arg1.lng = arg1; + instr_ptr(ctx, ret)->arg2.uint = arg2; + return S_OK; +} + static HRESULT push_instr_addr(compile_ctx_t *ctx, vbsop_t op, unsigned arg) { unsigned ret; @@ -1199,6 +1212,21 @@ static HRESULT compile_assignment(compile_ctx_t *ctx, expression_t *left, expres return hres; } + if(!member_expr->obj_expr) { + int local_ref; + if(bind_local(ctx, member_expr->identifier, &local_ref)) { + hres = push_instr_int_uint(ctx, is_set ? OP_set_local : OP_assign_local, + local_ref, args_cnt); + if(FAILED(hres)) + return hres; + + if(!emit_catch(ctx, 0)) + return E_OUTOFMEMORY; + + return S_OK; + } + } + hres = push_instr_bstr_uint(ctx, op, member_expr->identifier, args_cnt); if(FAILED(hres)) return hres; diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index af62c02680d..b87573674d7 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -1070,6 +1070,52 @@ static HRESULT interp_set_ident(exec_ctx_t *ctx) return S_OK; } +static HRESULT interp_assign_local(exec_ctx_t *ctx) +{ + const int ref = ctx->instr->arg1.lng; + const unsigned arg_cnt = ctx->instr->arg2.uint; + VARIANT *v; + DISPPARAMS dp; + HRESULT hres; + + v = ref < 0 ? ctx->args - ref - 1 : ctx->vars + ref; + + TRACE("%d\n", ref); + + vbstack_to_dp(ctx, arg_cnt, TRUE, &dp); + hres = assign_local_var(ctx, v, DISPATCH_PROPERTYPUT, &dp); + if(FAILED(hres)) + return hres; + + stack_popn(ctx, arg_cnt+1); + return S_OK; +} + +static HRESULT interp_set_local(exec_ctx_t *ctx) +{ + const int ref = ctx->instr->arg1.lng; + const unsigned arg_cnt = ctx->instr->arg2.uint; + VARIANT *v; + DISPPARAMS dp; + HRESULT hres; + + v = ref < 0 ? ctx->args - ref - 1 : ctx->vars + ref; + + TRACE("%d %u\n", ref, arg_cnt); + + hres = stack_assume_disp(ctx, arg_cnt, NULL); + if(FAILED(hres)) + return hres; + + vbstack_to_dp(ctx, arg_cnt, TRUE, &dp); + hres = assign_local_var(ctx, v, DISPATCH_PROPERTYPUTREF, &dp); + if(FAILED(hres)) + return hres; + + stack_popn(ctx, arg_cnt + 1); + return S_OK; +} + static HRESULT interp_assign_member(exec_ctx_t *ctx) { BSTR identifier = ctx->instr->arg1.bstr; diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 353aab3a9e3..53fcb3bb518 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -241,6 +241,7 @@ typedef enum { X(add, 1, 0, 0) \ X(and, 1, 0, 0) \ X(assign_ident, 1, ARG_BSTR, ARG_UINT) \ + X(assign_local, 1, ARG_INT, ARG_UINT) \ X(assign_member, 1, ARG_BSTR, ARG_UINT) \ X(bool, 1, ARG_INT, 0) \ X(catch, 1, ARG_ADDR, ARG_UINT) \ @@ -296,6 +297,7 @@ typedef enum { X(ret, 0, 0, 0) \ X(retval, 1, 0, 0) \ X(set_ident, 1, ARG_BSTR, ARG_UINT) \ + X(set_local, 1, ARG_INT, ARG_UINT) \ X(set_member, 1, ARG_BSTR, ARG_UINT) \ X(stack, 1, ARG_UINT, 0) \ X(step, 0, ARG_ADDR, ARG_BSTR) \ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10515
From: Francis De Brabandere <francisdb@gmail.com> Extract the common bodies so that future compile-time-bound For-loop operations on local variables can reuse them instead of duplicating the logic. No functional change. --- dlls/vbscript/interp.c | 84 ++++++++++++++++++++++++------------------ 1 file changed, 49 insertions(+), 35 deletions(-) diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index b87573674d7..63e80705c96 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -1648,11 +1648,42 @@ static HRESULT interp_erase(exec_ctx_t *ctx) return hres; } -static HRESULT interp_step(exec_ctx_t *ctx) +static HRESULT do_for_step(exec_ctx_t *ctx, VARIANT *loop_var) { - const BSTR ident = ctx->instr->arg2.bstr; BOOL gteq_zero; VARIANT zero; + HRESULT hres; + + V_VT(&zero) = VT_I2; + V_I2(&zero) = 0; + hres = VarCmp(stack_top(ctx, 0), &zero, ctx->script->lcid, 0); + if(FAILED(hres)) + goto loop_not_initialized; + + gteq_zero = hres == VARCMP_GT || hres == VARCMP_EQ; + + hres = VarCmp(loop_var, stack_top(ctx, 1), ctx->script->lcid, 0); + if(FAILED(hres)) + goto loop_not_initialized; + + if(hres == VARCMP_EQ || hres == (gteq_zero ? VARCMP_LT : VARCMP_GT)) { + ctx->instr++; + }else { + stack_popn(ctx, 3); + instr_jmp(ctx, ctx->instr->arg1.uint); + } + return S_OK; + +loop_not_initialized: + WARN("For loop not initialized\n"); + stack_popn(ctx, 2); + instr_jmp(ctx, ctx->instr->arg1.uint); + return hres; +} + +static HRESULT interp_step(exec_ctx_t *ctx) +{ + const BSTR ident = ctx->instr->arg2.bstr; ref_t ref; HRESULT hres; @@ -1671,14 +1702,6 @@ static HRESULT interp_step(exec_ctx_t *ctx) return S_OK; } - V_VT(&zero) = VT_I2; - V_I2(&zero) = 0; - hres = VarCmp(stack_top(ctx, 0), &zero, ctx->script->lcid, 0); - if(FAILED(hres)) - goto loop_not_initialized; - - gteq_zero = hres == VARCMP_GT || hres == VARCMP_EQ; - hres = lookup_identifier(ctx, ident, VBDISP_ANY, &ref); if(FAILED(hres)) return hres; @@ -1688,23 +1711,7 @@ static HRESULT interp_step(exec_ctx_t *ctx) return E_FAIL; } - hres = VarCmp(ref.u.v, stack_top(ctx, 1), ctx->script->lcid, 0); - if(FAILED(hres)) - goto loop_not_initialized; - - if(hres == VARCMP_EQ || hres == (gteq_zero ? VARCMP_LT : VARCMP_GT)) { - ctx->instr++; - }else { - stack_popn(ctx, 3); - instr_jmp(ctx, ctx->instr->arg1.uint); - } - return S_OK; - -loop_not_initialized: - WARN("For loop not initialized\n"); - stack_popn(ctx, 2); - instr_jmp(ctx, ctx->instr->arg1.uint); - return hres; + return do_for_step(ctx, ref.u.v); } static HRESULT interp_newenum(exec_ctx_t *ctx) @@ -2608,10 +2615,23 @@ static HRESULT interp_neg(exec_ctx_t *ctx) return stack_push(ctx, &v); } +static HRESULT do_incc(exec_ctx_t *ctx, VARIANT *v) +{ + VARIANT tmp; + HRESULT hres; + + hres = VarAdd(stack_top(ctx, 0), v, &tmp); + if(FAILED(hres)) + return hres; + + VariantClear(v); + *v = tmp; + return S_OK; +} + static HRESULT interp_incc(exec_ctx_t *ctx) { const BSTR ident = ctx->instr->arg1.bstr; - VARIANT v; ref_t ref; HRESULT hres; @@ -2626,13 +2646,7 @@ static HRESULT interp_incc(exec_ctx_t *ctx) return E_FAIL; } - hres = VarAdd(stack_top(ctx, 0), ref.u.v, &v); - if(FAILED(hres)) - return hres; - - VariantClear(ref.u.v); - *ref.u.v = v; - return S_OK; + return do_incc(ctx, ref.u.v); } static HRESULT interp_with(exec_ctx_t *ctx) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10515
From: Francis De Brabandere <francisdb@gmail.com> Emit OP_step_local and OP_incc_local instead of OP_step and OP_incc when the loop counter is a known local variable or function argument. This eliminates runtime lookup_identifier() calls on every loop iteration. This mirrors jscript's approach of resolving locals at compile time. --- dlls/vbscript/compile.c | 37 ++++++++++++++++++++++++++++--------- dlls/vbscript/interp.c | 31 +++++++++++++++++++++++++++++++ dlls/vbscript/vbscript.h | 2 ++ 3 files changed, 61 insertions(+), 9 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 890d024a276..f2abed5cc82 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -917,8 +917,12 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st statement_ctx_t loop_ctx = {3}; unsigned step_instr, instr, expr_err_label, past_err_label, body_label, from_offset; BSTR identifier; + int local_ref; + BOOL is_local; HRESULT hres; + is_local = bind_local(ctx, stat->identifier, &local_ref); + identifier = alloc_bstr_arg(ctx, stat->identifier); if(!identifier) return E_OUTOFMEMORY; @@ -987,10 +991,17 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st if(!loop_ctx.for_end_label) return E_OUTOFMEMORY; - step_instr = push_instr(ctx, OP_step); - if(!step_instr) - return E_OUTOFMEMORY; - instr_ptr(ctx, step_instr)->arg2.bstr = identifier; + if(is_local) { + step_instr = push_instr(ctx, OP_step_local); + if(!step_instr) + return E_OUTOFMEMORY; + instr_ptr(ctx, step_instr)->arg2.lng = local_ref; + }else { + step_instr = push_instr(ctx, OP_step); + if(!step_instr) + return E_OUTOFMEMORY; + instr_ptr(ctx, step_instr)->arg2.bstr = identifier; + } instr_ptr(ctx, step_instr)->arg1.uint = loop_ctx.for_end_label; if(!emit_catch(ctx, 3)) @@ -1002,11 +1013,19 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st if(FAILED(hres)) return hres; - - instr = push_instr(ctx, OP_incc); - if(!instr) - return E_OUTOFMEMORY; - instr_ptr(ctx, instr)->arg1.bstr = identifier; + /* We need a separated OP_step here so that errors jump to the end-of-loop catch. */ + ctx->loc = stat->stat.loc; + if(is_local) { + instr = push_instr(ctx, OP_incc_local); + if(!instr) + return E_OUTOFMEMORY; + instr_ptr(ctx, instr)->arg1.lng = local_ref; + }else { + instr = push_instr(ctx, OP_incc); + if(!instr) + return E_OUTOFMEMORY; + instr_ptr(ctx, instr)->arg1.bstr = identifier; + } instr = push_instr(ctx, OP_step); if(!instr) diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index 63e80705c96..d89904fb9d5 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -1714,6 +1714,28 @@ static HRESULT interp_step(exec_ctx_t *ctx) return do_for_step(ctx, ref.u.v); } +static HRESULT interp_step_local(exec_ctx_t *ctx) +{ + const int ref = ctx->instr->arg2.lng; + VARIANT *v; + + TRACE("%d\n", ref); + + v = ref < 0 ? ctx->args - ref - 1 : ctx->vars + ref; + + if(V_VT(stack_top(ctx, 0)) == VT_EMPTY && V_VT(stack_top(ctx, 1)) == VT_EMPTY) { + WARN("For loop not initialized\n"); + clear_ei(&ctx->script->ei); + ctx->script->ei.scode = MAKE_VBSERROR(VBSE_FOR_LOOP_NOT_INITIALIZED); + map_vbs_exception(&ctx->script->ei); + stack_popn(ctx, 3); + instr_jmp(ctx, ctx->instr->arg1.uint); + return S_OK; + } + + return do_for_step(ctx, v); +} + static HRESULT interp_newenum(exec_ctx_t *ctx) { variant_val_t v; @@ -2649,6 +2671,15 @@ static HRESULT interp_incc(exec_ctx_t *ctx) return do_incc(ctx, ref.u.v); } +static HRESULT interp_incc_local(exec_ctx_t *ctx) +{ + const int ref = ctx->instr->arg1.lng; + + TRACE("%d\n", ref); + + return do_incc(ctx, ref < 0 ? ctx->args - ref - 1 : ctx->vars + ref); +} + static HRESULT interp_with(exec_ctx_t *ctx) { VARIANT *v; diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 53fcb3bb518..73c1841823d 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -269,6 +269,7 @@ typedef enum { X(idiv, 1, 0, 0) \ X(imp, 1, 0, 0) \ X(incc, 1, ARG_BSTR, 0) \ + X(incc_local, 1, ARG_INT, 0) \ X(int, 1, ARG_INT, 0) \ X(is, 1, 0, 0) \ X(local, 1, ARG_INT, 0) \ @@ -301,6 +302,7 @@ typedef enum { X(set_member, 1, ARG_BSTR, ARG_UINT) \ X(stack, 1, ARG_UINT, 0) \ X(step, 0, ARG_ADDR, ARG_BSTR) \ + X(step_local, 0, ARG_ADDR, ARG_INT) \ X(stop, 1, 0, 0) \ X(string, 1, ARG_STR, 0) \ X(sub, 1, 0, 0) \ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10515
From: Francis De Brabandere <francisdb@gmail.com> Resolve class property names to direct indices during compilation of class methods, emitting OP_local_prop, OP_assign_local_prop, and OP_set_local_prop instead of the string-based OP_ident/OP_assign_ident/ OP_set_ident. This addresses the FIXME in lookup_identifier() that noted these should be bound while generating bytecode. This mirrors jscript's approach of resolving identifiers at compile time rather than doing runtime string comparisons. --- dlls/vbscript/compile.c | 54 +++++++++++++++++++++++++++++++++++++++ dlls/vbscript/interp.c | 55 ++++++++++++++++++++++++++++++++++++++++ dlls/vbscript/vbscript.h | 11 +++++--- 3 files changed, 116 insertions(+), 4 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index f2abed5cc82..c969b9a0058 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -63,6 +63,7 @@ typedef struct { function_t *func; function_decl_t *func_decls; + dim_decl_t *class_props; } compile_ctx_t; static HRESULT compile_expression(compile_ctx_t*,expression_t*); @@ -205,6 +206,19 @@ static HRESULT push_instr_int_uint(compile_ctx_t *ctx, vbsop_t op, LONG arg1, un return S_OK; } +static HRESULT push_instr_uint_uint(compile_ctx_t *ctx, vbsop_t op, unsigned arg1, unsigned arg2) +{ + unsigned ret; + + ret = push_instr(ctx, op); + if(!ret) + return E_OUTOFMEMORY; + + instr_ptr(ctx, ret)->arg1.uint = arg1; + instr_ptr(ctx, ret)->arg2.uint = arg2; + return S_OK; +} + static HRESULT push_instr_addr(compile_ctx_t *ctx, vbsop_t op, unsigned arg) { unsigned ret; @@ -497,6 +511,26 @@ static BOOL bind_local(compile_ctx_t *ctx, const WCHAR *name, int *ret) return FALSE; } +/* Resolve an identifier to a class property index at compile time. + * Only valid when compiling inside a class method (ctx->class_props != NULL). */ +static BOOL bind_class_prop(compile_ctx_t *ctx, const WCHAR *name, unsigned *ret) +{ + dim_decl_t *prop; + unsigned i; + + if(!ctx->class_props) + return FALSE; + + for(prop = ctx->class_props, i = 0; prop; prop = prop->next, i++) { + if(!vbs_wcsicmp(prop->name, name)) { + *ret = i; + return TRUE; + } + } + + return FALSE; +} + static HRESULT compile_args(compile_ctx_t *ctx, expression_t *args, unsigned *ret) { unsigned arg_cnt = 0; @@ -547,6 +581,7 @@ static HRESULT compile_member_call_expression(compile_ctx_t *ctx, member_express static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t *expr) { expression_t *const_expr; + unsigned prop_ref; int local_ref; if (expr->obj_expr) /* FIXME: we should probably have a dedicated opcode as well */ @@ -555,6 +590,9 @@ static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t if(bind_local(ctx, expr->identifier, &local_ref)) return push_instr_int(ctx, OP_local, local_ref); + if(bind_class_prop(ctx, expr->identifier, &prop_ref)) + return push_instr_uint(ctx, OP_local_prop, prop_ref); + const_expr = lookup_const_decls(ctx, expr->identifier, TRUE); if(const_expr) return compile_expression(ctx, const_expr); @@ -1233,6 +1271,7 @@ static HRESULT compile_assignment(compile_ctx_t *ctx, expression_t *left, expres if(!member_expr->obj_expr) { int local_ref; + unsigned prop_ref; if(bind_local(ctx, member_expr->identifier, &local_ref)) { hres = push_instr_int_uint(ctx, is_set ? OP_set_local : OP_assign_local, local_ref, args_cnt); @@ -1244,6 +1283,17 @@ static HRESULT compile_assignment(compile_ctx_t *ctx, expression_t *left, expres return S_OK; } + if(bind_class_prop(ctx, member_expr->identifier, &prop_ref)) { + hres = push_instr_uint_uint(ctx, is_set ? OP_set_local_prop : OP_assign_local_prop, + prop_ref, args_cnt); + if(FAILED(hres)) + return hres; + + if(!emit_catch(ctx, 0)) + return E_OUTOFMEMORY; + + return S_OK; + } } hres = push_instr_bstr_uint(ctx, op, member_expr->identifier, args_cnt); @@ -2023,6 +2073,8 @@ static HRESULT compile_class(compile_ctx_t *ctx, class_decl_t *class_decl) return E_OUTOFMEMORY; memset(class_desc->funcs, 0, class_desc->func_cnt*sizeof(*class_desc->funcs)); + ctx->class_props = class_decl->props; + for(func_decl = class_decl->funcs, i=1; func_decl; func_decl = func_decl->next, i++) { for(func_prop_decl = func_decl; func_prop_decl; func_prop_decl = func_prop_decl->next_prop_func) { if(func_prop_decl->is_default) { @@ -2060,6 +2112,8 @@ static HRESULT compile_class(compile_ctx_t *ctx, class_decl_t *class_decl) return hres; } + ctx->class_props = NULL; + for(prop_decl = class_decl->props; prop_decl; prop_decl = prop_decl->next) class_desc->prop_cnt++; diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index d89904fb9d5..0196901573a 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -894,6 +894,21 @@ static HRESULT interp_local(exec_ctx_t *ctx) return stack_push(ctx, &r); } +static HRESULT interp_local_prop(exec_ctx_t *ctx) +{ + const unsigned arg = ctx->instr->arg1.uint; + VARIANT *v; + VARIANT r; + + v = ctx->vbthis->props + arg; + + TRACE("%s\n", debugstr_variant(v)); + + V_VT(&r) = VT_BYREF|VT_VARIANT; + V_BYREF(&r) = v; + return stack_push(ctx, &r); +} + static HRESULT interp_ident(exec_ctx_t *ctx) { BSTR identifier = ctx->instr->arg1.bstr; @@ -1116,6 +1131,46 @@ static HRESULT interp_set_local(exec_ctx_t *ctx) return S_OK; } +static HRESULT interp_assign_local_prop(exec_ctx_t *ctx) +{ + const unsigned ref = ctx->instr->arg1.uint; + const unsigned arg_cnt = ctx->instr->arg2.uint; + DISPPARAMS dp; + HRESULT hres; + + TRACE("%u\n", ref); + + vbstack_to_dp(ctx, arg_cnt, TRUE, &dp); + hres = assign_local_var(ctx, ctx->vbthis->props + ref, DISPATCH_PROPERTYPUT, &dp); + if(FAILED(hres)) + return hres; + + stack_popn(ctx, arg_cnt+1); + return S_OK; +} + +static HRESULT interp_set_local_prop(exec_ctx_t *ctx) +{ + const unsigned ref = ctx->instr->arg1.uint; + const unsigned arg_cnt = ctx->instr->arg2.uint; + DISPPARAMS dp; + HRESULT hres; + + TRACE("%u %u\n", ref, arg_cnt); + + hres = stack_assume_disp(ctx, arg_cnt, NULL); + if(FAILED(hres)) + return hres; + + vbstack_to_dp(ctx, arg_cnt, TRUE, &dp); + hres = assign_local_var(ctx, ctx->vbthis->props + ref, DISPATCH_PROPERTYPUTREF, &dp); + if(FAILED(hres)) + return hres; + + stack_popn(ctx, arg_cnt + 1); + return S_OK; +} + static HRESULT interp_assign_member(exec_ctx_t *ctx) { BSTR identifier = ctx->instr->arg1.bstr; diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 73c1841823d..1bc3f788577 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -240,9 +240,10 @@ typedef enum { #define OP_LIST \ X(add, 1, 0, 0) \ X(and, 1, 0, 0) \ - X(assign_ident, 1, ARG_BSTR, ARG_UINT) \ - X(assign_local, 1, ARG_INT, ARG_UINT) \ - X(assign_member, 1, ARG_BSTR, ARG_UINT) \ + X(assign_ident, 1, ARG_BSTR, ARG_UINT) \ + X(assign_local, 1, ARG_INT, ARG_UINT) \ + X(assign_local_prop, 1, ARG_UINT, ARG_UINT) \ + X(assign_member, 1, ARG_BSTR, ARG_UINT) \ X(bool, 1, ARG_INT, 0) \ X(catch, 1, ARG_ADDR, ARG_UINT) \ X(case, 0, ARG_ADDR, 0) \ @@ -272,10 +273,11 @@ typedef enum { X(incc_local, 1, ARG_INT, 0) \ X(int, 1, ARG_INT, 0) \ X(is, 1, 0, 0) \ - X(local, 1, ARG_INT, 0) \ X(jmp, 0, ARG_ADDR, 0) \ X(jmp_false, 0, ARG_ADDR, 0) \ X(jmp_true, 0, ARG_ADDR, 0) \ + X(local, 1, ARG_INT, 0) \ + X(local_prop, 1, ARG_UINT, 0) \ X(lt, 1, 0, 0) \ X(lteq, 1, 0, 0) \ X(mcall, 1, ARG_BSTR, ARG_UINT) \ @@ -299,6 +301,7 @@ typedef enum { X(retval, 1, 0, 0) \ X(set_ident, 1, ARG_BSTR, ARG_UINT) \ X(set_local, 1, ARG_INT, ARG_UINT) \ + X(set_local_prop, 1, ARG_UINT, ARG_UINT) \ X(set_member, 1, ARG_BSTR, ARG_UINT) \ X(stack, 1, ARG_UINT, 0) \ X(step, 0, ARG_ADDR, ARG_BSTR) \ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10515
On Fri Apr 17 14:09:57 2026 +0000, Jacek Caban wrote:
In general, I prefer keeping related commits grouped in MRs. Having many small MRs with various dependencies or conflicts makes things harder to track, so keeping a few commits per MR is usually the best approach. In this particular case, moving the more controversial parts of this MR, namely the SAFEARRAY special case and global code handling, out of this MR would help move it forward, so that seems reasonable. Ok, removed related commits, now doing to do some measurements to prove that all this is worth it and update the description.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10515#note_136806
With current master this whole effort only yields 5% improvement in synthetic benchmarks. !10546 and !10544 seem to have been the main issues However with the other performance related MR's this might change. I built these on a big wip performance branch and then pulled them out. So, waiting for !10541, !10528, !10546 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10515#note_136845
participants (2)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)