- optimized global variable access

- removed full compliance with the spec for strict mode variable
  assignment so that they are as fast as in non strict mode (V8,
  SpiderMonkey and JavascriptCore do the same, so IMHO the spec should
  be updated).

Author: Fabrice Bellard

quickjs-opcode.h

@@ -123,12 +123,10 @@ DEF(         regexp, 1, 2, 1, none) /* create a RegExp object from the pattern a
 DEF(      get_super, 1, 1, 1, none)
 DEF(         import, 1, 2, 1, none) /* dynamic module import */
 
-DEF(      check_var, 5, 0, 1, atom) /* check if a variable exists */
 DEF(  get_var_undef, 5, 0, 1, atom) /* push undefined if the variable does not exist */
 DEF(        get_var, 5, 0, 1, atom) /* throw an exception if the variable does not exist */
 DEF(        put_var, 5, 1, 0, atom) /* must come after get_var */
 DEF(   put_var_init, 5, 1, 0, atom) /* must come after put_var. Used to initialize a global lexical variable */
-DEF( put_var_strict, 5, 2, 0, atom) /* for strict mode variable write */
 
 DEF(  get_ref_value, 1, 2, 3, none)
 DEF(  put_ref_value, 1, 3, 0, none)

quickjs.c

@@ -10175,8 +10175,8 @@ static int JS_DefineGlobalFunction(JSContext *ctx, JSAtom prop,
     return 0;
 }
 
-static JSValue JS_GetGlobalVar(JSContext *ctx, JSAtom prop,
-                               BOOL throw_ref_error)
+static inline JSValue JS_GetGlobalVar(JSContext *ctx, JSAtom prop,
+                                      BOOL throw_ref_error)
 {
     JSObject *p;
     JSShapeProperty *prs;
@@ -10191,6 +10191,14 @@ static JSValue JS_GetGlobalVar(JSContext *ctx, JSAtom prop,
             return JS_ThrowReferenceErrorUninitialized(ctx, prs->atom);
         return JS_DupValue(ctx, pr->u.value);
     }
+
+    /* fast path */
+    p = JS_VALUE_GET_OBJ(ctx->global_obj);
+    prs = find_own_property(&pr, p, prop);
+    if (prs) {
+        if (likely((prs->flags & JS_PROP_TMASK) == 0))
+            return JS_DupValue(ctx, pr->u.value);
+    }
     return JS_GetPropertyInternal(ctx, ctx->global_obj, prop,
                                  ctx->global_obj, throw_ref_error);
 }
@@ -10232,37 +10240,16 @@ static int JS_GetGlobalVarRef(JSContext *ctx, JSAtom prop, JSValue *sp)
     return 0;
 }
 
-/* use for strict variable access: test if the variable exists */
-static int JS_CheckGlobalVar(JSContext *ctx, JSAtom prop)
-{
-    JSObject *p;
-    JSShapeProperty *prs;
-    int ret;
-
-    /* no exotic behavior is possible in global_var_obj */
-    p = JS_VALUE_GET_OBJ(ctx->global_var_obj);
-    prs = find_own_property1(p, prop);
-    if (prs) {
-        ret = TRUE;
-    } else {
-        ret = JS_HasProperty(ctx, ctx->global_obj, prop);
-        if (ret < 0)
-            return -1;
-    }
-    return ret;
-}
-
 /* flag = 0: normal variable write
    flag = 1: initialize lexical variable
-   flag = 2: normal variable write, strict check was done before
 */
-static int JS_SetGlobalVar(JSContext *ctx, JSAtom prop, JSValue val,
-                           int flag)
+static inline int JS_SetGlobalVar(JSContext *ctx, JSAtom prop, JSValue val,
+                                  int flag)
 {
     JSObject *p;
     JSShapeProperty *prs;
     JSProperty *pr;
-    int flags;
+    int ret;
 
     /* no exotic behavior is possible in global_var_obj */
     p = JS_VALUE_GET_OBJ(ctx->global_var_obj);
@@ -10283,13 +10270,30 @@ static int JS_SetGlobalVar(JSContext *ctx, JSAtom prop, JSValue val,
         set_value(ctx, &pr->u.value, val);
         return 0;
     }
-    /* XXX: add a fast path where the property exists and the object
-       is not exotic. Otherwise do as in OP_put_ref_value and remove
-       JS_PROP_NO_ADD which is no longer necessary */
-    flags = JS_PROP_THROW_STRICT;
-    if (is_strict_mode(ctx))
-        flags |= JS_PROP_NO_ADD;
-    return JS_SetPropertyInternal(ctx, ctx->global_obj, prop, val, ctx->global_obj, flags);
+    
+    p = JS_VALUE_GET_OBJ(ctx->global_obj);
+    prs = find_own_property(&pr, p, prop);
+    if (prs) {
+        if (likely((prs->flags & (JS_PROP_TMASK | JS_PROP_WRITABLE |
+                                  JS_PROP_LENGTH)) == JS_PROP_WRITABLE)) {
+            /* fast path */
+            set_value(ctx, &pr->u.value, val);
+            return 0;
+        }
+    }
+    /* slow path */
+    ret = JS_HasProperty(ctx, ctx->global_obj, prop);
+    if (ret < 0) {
+        JS_FreeValue(ctx, val);
+        return -1;
+    }
+    if (ret == 0 && is_strict_mode(ctx)) {
+        JS_FreeValue(ctx, val);
+        JS_ThrowReferenceErrorNotDefined(ctx, prop);
+        return -1;
+    }
+    return JS_SetPropertyInternal(ctx, ctx->global_obj, prop, val, ctx->global_obj,
+                                  JS_PROP_THROW_STRICT);
 }
 
 /* return -1, FALSE or TRUE */
@@ -17597,21 +17601,6 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
             }
             BREAK;
 
-        CASE(OP_check_var):
-            {
-                int ret;
-                JSAtom atom;
-                atom = get_u32(pc);
-                pc += 4;
-                sf->cur_pc = pc;
-
-                ret = JS_CheckGlobalVar(ctx, atom);
-                if (ret < 0)
-                    goto exception;
-                *sp++ = JS_NewBool(ctx, ret);
-            }
-            BREAK;
-
         CASE(OP_get_var_undef):
         CASE(OP_get_var):
             {
@@ -17644,26 +17633,6 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValueConst func_obj,
             }
             BREAK;
 
-        CASE(OP_put_var_strict):
-            {
-                int ret;
-                JSAtom atom;
-                atom = get_u32(pc);
-                pc += 4;
-                sf->cur_pc = pc;
-
-                /* sp[-2] is JS_TRUE or JS_FALSE */
-                if (unlikely(!JS_VALUE_GET_INT(sp[-2]))) {
-                    JS_ThrowReferenceErrorNotDefined(ctx, atom);
-                    goto exception;
-                }
-                ret = JS_SetGlobalVar(ctx, atom, sp[-1], 2);
-                sp -= 2;
-                if (unlikely(ret < 0))
-                    goto exception;
-            }
-            BREAK;
-
         CASE(OP_check_define_var):
             {
                 JSAtom atom;
@@ -31578,20 +31547,10 @@ static int optimize_scope_make_global_ref(JSContext *ctx, JSFunctionDef *s,
                                           JSAtom var_name)
 {
     int label_pos, end_pos, pos, op;
-    BOOL is_strict;
-    is_strict = ((s->js_mode & JS_MODE_STRICT) != 0);
 
     /* replace the reference get/put with normal variable
        accesses */
-    if (is_strict) {
-        /* need to check if the variable exists before evaluating the right
-           expression */
-        /* XXX: need an extra OP_true if destructuring an array */
-        dbuf_putc(bc, OP_check_var);
-        dbuf_put_u32(bc, JS_DupAtom(ctx, var_name));
-    } else {
-        /* XXX: need 2 extra OP_true if destructuring an array */
-    }
+    /* XXX: need 2 extra OP_true if destructuring an array */
     if (bc_buf[pos_next] == OP_get_ref_value) {
         dbuf_putc(bc, OP_get_var);
         dbuf_put_u32(bc, JS_DupAtom(ctx, var_name));
@@ -31605,34 +31564,10 @@ static int optimize_scope_make_global_ref(JSContext *ctx, JSFunctionDef *s,
     assert(bc_buf[pos] == OP_label);
     end_pos = label_pos + 2;
     op = bc_buf[label_pos];
-    if (is_strict) {
-        if (op != OP_nop) {
-            switch(op) {
-            case OP_insert3:
-                op = OP_insert2;
-                break;
-            case OP_perm4:
-                op = OP_perm3;
-                break;
-            case OP_rot3l:
-                op = OP_swap;
-                break;
-            default:
-                abort();
-            }
-            bc_buf[pos++] = op;
-        }
-    } else {
-        if (op == OP_insert3)
-            bc_buf[pos++] = OP_dup;
-    }
-    if (is_strict) {
-        bc_buf[pos] = OP_put_var_strict;
-        /* XXX: need 1 extra OP_drop if destructuring an array */
-    } else {
-        bc_buf[pos] = OP_put_var;
-        /* XXX: need 2 extra OP_drop if destructuring an array */
-    }
+    if (op == OP_insert3)
+        bc_buf[pos++] = OP_dup;
+    bc_buf[pos] = OP_put_var;
+    /* XXX: need 2 extra OP_drop if destructuring an array */
     put_u32(bc_buf + pos + 1, JS_DupAtom(ctx, var_name));
     pos += 5;
     /* pad with OP_nop */
@@ -34103,12 +34038,11 @@ static __exception int resolve_labels(JSContext *ctx, JSFunctionDef *s)
             if (OPTIMIZE) {
                 /* Transformation:
                    insert2 put_field(a) drop -> put_field(a)
-                   insert2 put_var_strict(a) drop -> put_var_strict(a)
                 */
-                if (code_match(&cc, pos_next, M2(OP_put_field, OP_put_var_strict), OP_drop, -1)) {
+                if (code_match(&cc, pos_next, OP_put_field, OP_drop, -1)) {
                     if (cc.line_num >= 0) line_num = cc.line_num;
                     add_pc2line_info(s, bc_out.size, line_num);
-                    dbuf_putc(&bc_out, cc.op);
+                    dbuf_putc(&bc_out, OP_put_field);
                     dbuf_put_u32(&bc_out, cc.atom);
                     pos_next = cc.pos;
                     break;
@@ -34254,7 +34188,6 @@ static __exception int resolve_labels(JSContext *ctx, JSFunctionDef *s)
                 /* transformation:
                    post_inc put_x drop -> inc put_x
                    post_inc perm3 put_field drop -> inc put_field
-                   post_inc perm3 put_var_strict drop -> inc put_var_strict
                    post_inc perm4 put_array_el drop -> inc put_array_el
                  */
                 int op1, idx;
@@ -34273,11 +34206,11 @@ static __exception int resolve_labels(JSContext *ctx, JSFunctionDef *s)
                     put_short_code(&bc_out, op1, idx);
                     break;
                 }
-                if (code_match(&cc, pos_next, OP_perm3, M2(OP_put_field, OP_put_var_strict), OP_drop, -1)) {
+                if (code_match(&cc, pos_next, OP_perm3, OP_put_field, OP_drop, -1)) {
                     if (cc.line_num >= 0) line_num = cc.line_num;
                     add_pc2line_info(s, bc_out.size, line_num);
                     dbuf_putc(&bc_out, OP_dec + (op - OP_post_dec));
-                    dbuf_putc(&bc_out, cc.op);
+                    dbuf_putc(&bc_out, OP_put_field);
                     dbuf_put_u32(&bc_out, cc.atom);
                     pos_next = cc.pos;
                     break;

test262_errors.txt

@@ -5,6 +5,7 @@ test262/test/annexB/language/expressions/assignmenttargettype/callexpression-in-
 test262/test/annexB/language/expressions/assignmenttargettype/callexpression-in-prefix-update.js:27: SyntaxError: invalid increment/decrement operand
 test262/test/annexB/language/expressions/assignmenttargettype/callexpression.js:33: SyntaxError: invalid assignment left-hand side
 test262/test/annexB/language/expressions/assignmenttargettype/cover-callexpression-and-asyncarrowhead.js:20: SyntaxError: invalid assignment left-hand side
+test262/test/language/identifier-resolution/assign-to-global-undefined.js:20: strict mode: expected error
 test262/test/staging/sm/Date/UTC-convert-all-arguments.js:13: Test262Error: index 1: expected 42, got Error: didn't throw Expected SameValue(«Error: didn't throw», «42») to be true
 test262/test/staging/sm/Date/UTC-convert-all-arguments.js:13: strict mode: Test262Error: index 1: expected 42, got Error: didn't throw Expected SameValue(«Error: didn't throw», «42») to be true
 test262/test/staging/sm/Date/constructor-convert-all-arguments.js:13: Test262Error: index undefined: expected 42, got Error: didn't throw Expected SameValue(«Error: didn't throw», «42») to be true

tests/microbench.js

@@ -527,22 +527,21 @@ function global_read(n)
     return n * 4;
 }
 
-// non strict version
-var global_write =
-    (1, eval)(`(function global_write(n)
-           {
-               var j;
-               for(j = 0; j < n; j++) {
-                   global_var0 = j;
-                   global_var0 = j;
-                   global_var0 = j;
-                   global_var0 = j;
-               }
-               return n * 4;
-           })`);
+function global_write(n)
+{
+    var j;
+    for(j = 0; j < n; j++) {
+        global_var0 = j;
+        global_var0 = j;
+        global_var0 = j;
+        global_var0 = j;
+    }
+    return n * 4;
+}
 
 function global_write_strict(n)
 {
+    "use strict";
     var j;
     for(j = 0; j < n; j++) {
         global_var0 = j;
@@ -570,23 +569,22 @@ function local_destruct(n)
 var global_v1, global_v2, global_v3, global_v4;
 var global_a, global_b, global_c, global_d;
 
-// non strict version
-var global_destruct =
-    (1, eval)(`(function global_destruct(n)
-           {
-               var j, v1, v2, v3, v4;
-               var array = [ 1, 2, 3, 4, 5 ];
-               var o = { a:1, b:2, c:3, d:4 };
-               var a, b, c, d;
-               for(j = 0; j < n; j++) {
-                   [ global_v1, global_v2,, global_v3, ...global_v4] = array;
-                   ({ a: global_a, b: global_b, c: global_c, d: global_d } = o);
-               }
-               return n * 8;
-          })`);
+function global_destruct(n)
+{
+    var j, v1, v2, v3, v4;
+    var array = [ 1, 2, 3, 4, 5 ];
+    var o = { a:1, b:2, c:3, d:4 };
+    var a, b, c, d;
+    for(j = 0; j < n; j++) {
+        [ global_v1, global_v2,, global_v3, ...global_v4] = array;
+        ({ a: global_a, b: global_b, c: global_c, d: global_d } = o);
+    }
+    return n * 8;
+}
 
 function global_destruct_strict(n)
 {
+    "use strict";
     var j, v1, v2, v3, v4;
     var array = [ 1, 2, 3, 4, 5 ];
     var o = { a:1, b:2, c:3, d:4 };