510500 - Recording arguments[x] needs more trace-tests

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

Apologies in advance if gcov is lying to me. I'm new at this and wouldn't know! Here's what it says anyway:

     1324:10077:    if (STOBJ_GET_CLASS(obj) == &js_ArgumentsClass) {
        -:10078:        unsigned depth;
        1:10079:        JSStackFrame *afp = guardArguments(obj, obj_ins, &depth);
        1:10080:        if (afp) {
        1:10081:            uintN int_idx = JSVAL_TO_INT(idx);
        1:10082:            jsval* vp = &afp->argv[int_idx];
        1:10083:            if (idx_ins->isconstq()) {
        1:10084:                if (int_idx >= 0 && int_idx < afp->argc)
    #####:10085:                    v_ins = get(vp);
        -:10086:                else
        1:10087:                    v_ins = INS_VOID();
        -:10088:            } else {
        -:10089:                // If the index is not a constant expression, we generate LIR to load the value from
        -:10090:                // the native stack area. The guard on js_ArgumentClass above ensures the up-to-date
        -:10091:                // value has been written back to the native stack area.
    #####:10092:                idx_ins = makeNumberInt32(idx_ins);
    #####:10093:                if (int_idx >= 0 && int_idx < afp->argc) {
    #####:10094:                    JSTraceType type = getCoercedType(*vp);
        -:10095:
        -:10096:                    // Guard that the argument has the same type on trace as during recording.
        -:10097:                    LIns* typemap_ins;
    #####:10098:                    if (callDepth == depth) {
        -:10099:                        // In this case, we are in the same frame where the arguments object was created.
        -:10100:                        // The entry type map is not necessarily up-to-date, so we capture a new type map
        -:10101:                        // for this point in the code.
    #####:10102:                        unsigned stackSlots = NativeStackSlots(cx, 0 /* callDepth */);
    #####:10103:                        if (stackSlots * sizeof(JSTraceType) > LirBuffer::MAX_SKIP_PAYLOAD_SZB)
    #####:10104:                            ABORT_TRACE("|arguments| requires saving too much stack");
    #####:10105:                        JSTraceType* typemap = (JSTraceType*) lir->insSkip(stackSlots * sizeof(JSTraceType))->payload();
    #####:10106:                        DetermineTypesVisitor detVisitor(*this, typemap);
    #####:10107:                        VisitStackSlots(detVisitor, cx, 0);
    #####:10108:                        typemap_ins = INS_CONSTPTR(typemap + 2 /* callee, this */);
        -:10109:                    } else {
        -:10110:                        // In this case, we are in a deeper frame from where the arguments object was
        -:10111:                        // created. The type map at the point of the call out from the creation frame
        -:10112:                        // is accurate.
        -:10113:                        // Note: this relies on the assumption that we abort on setting an element of
        -:10114:                        // an arguments object in any deeper frame.
    #####:10115:                        LIns* fip_ins = lir->insLoad(LIR_ldp, lirbuf->rp, (callDepth-depth)*sizeof(FrameInfo*));
    #####:10116:                        typemap_ins = lir->ins2(LIR_add, fip_ins, INS_CONST(sizeof(FrameInfo) + 2/*callee,this*/ * sizeof(JSTraceType)));
        -:10117:                    }
        -:10118:
        -:10119:                    LIns* typep_ins = lir->ins2(LIR_add, typemap_ins,
    #####:10120:                                                lir->ins2(LIR_mul, idx_ins, INS_CONST(sizeof(JSTraceType))));
    #####:10121:                    LIns* type_ins = lir->insLoad(LIR_ldcb, typep_ins, 0);
        -:10122:                    guard(true,
        -:10123:                          addName(lir->ins2(LIR_eq, type_ins, lir->insImm(type)),
        -:10124:                                  "guard(type-stable upvar)"),
    #####:10125:                          BRANCH_EXIT);
        -:10126:
        -:10127:                    // Read the value out of the native stack area.
        -:10128:                    guard(true, lir->ins2(LIR_ult, idx_ins, INS_CONST(afp->argc)),
    #####:10129:                          snapshot(BRANCH_EXIT));
    #####:10130:                    size_t stackOffset = -treeInfo->nativeStackBase + nativeStackOffset(&afp->argv[0]);
    #####:10131:                    LIns* args_addr_ins = lir->ins2(LIR_add, lirbuf->sp, INS_CONST(stackOffset));
        -:10132:                    LIns* argi_addr_ins = lir->ins2(LIR_add, args_addr_ins,
    #####:10133:                                                    lir->ins2(LIR_mul, idx_ins, INS_CONST(sizeof(double))));
    #####:10134:                    v_ins = stackLoad(argi_addr_ins, type);
        -:10135:                } else {
        -:10136:                    guard(false, lir->ins2(LIR_ult, idx_ins, INS_CONST(afp->argc)),
    #####:10137:                          snapshot(BRANCH_EXIT));
    #####:10138:                    v_ins = INS_VOID();
        -:10139:                }
        -:10140:            }
        1:10141:            JS_ASSERT(v_ins);
        1:10142:            set(&lval, v_ins);
        1:10143:            return JSRS_CONTINUE;
        -:10144:        }
    #####:10145:        ABORT_TRACE("can't reach arguments object's frame");
        -:10146:    }

The #####s are lines that didn't execute in a 'make check' run.

Comment 1

[David Mandelin [:dmandelin]]

Jason, do you think the arguments directory I added recently takes care of this, or are we still missing basic blocks?

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Obsolete with the removal of tracejit.