Unreviewed, rolling out r174606.

https://bugs.webkit.org/show_bug.cgi?id=137621

broke a JSC test (Requested by estes on #webkit).

Reverted changeset:

"Various arguments optimisations in codegen fail to account
for arguments being in lexical record"
https://bugs.webkit.org/show_bug.cgi?id=137617
http://trac.webkit.org/changeset/174606

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@174610 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,17 @@
+2014-10-10  Commit Queue  <[email protected]>
+
+        Unreviewed, rolling out r174606.
+        https://bugs.webkit.org/show_bug.cgi?id=137621
+
+        broke a JSC test (Requested by estes on #webkit).
+
+        Reverted changeset:
+
+        "Various arguments optimisations in codegen fail to account
+        for arguments being in lexical record"
+        https://bugs.webkit.org/show_bug.cgi?id=137617
+        http://trac.webkit.org/changeset/174606
+
 2014-10-10  Oliver Hunt  <[email protected]>
 
         Various arguments optimisations in codegen fail to account for arguments being in lexical record

Source/JavaScriptCore/bytecode/CodeBlock.cpp

@@ -3887,8 +3887,6 @@ struct VerifyCapturedDef {
 
         if (codeBlock->usesArguments() && virtualReg == codeBlock->argumentsRegister())
             return;
-        if (codeBlock->usesArguments() && virtualReg == unmodifiedArgumentsRegister(codeBlock->argumentsRegister()))
-            return;
 
         if (codeBlock->captureCount() && codeBlock->symbolTable()->isCaptured(operand)) {
             codeBlock->beginValidationDidFail();

Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -167,7 +167,6 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, ProgramNode* programNode, UnlinkedP
     , m_lexicalEnvironmentRegister(0)
     , m_emptyValueRegister(0)
     , m_globalObjectRegister(0)
-    , m_localArgumentsRegister(0)
     , m_finallyDepth(0)
     , m_localScopeDepth(0)
     , m_codeType(GlobalCode)
@@ -212,7 +211,6 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionBodyNode* functionBody, Unl
     , m_lexicalEnvironmentRegister(0)
     , m_emptyValueRegister(0)
     , m_globalObjectRegister(0)
-    , m_localArgumentsRegister(0)
     , m_finallyDepth(0)
     , m_localScopeDepth(0)
     , m_codeType(FunctionCode)
@@ -252,16 +250,13 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionBodyNode* functionBody, Unl
         emitOpcode(op_create_lexical_environment);
         instructions().append(m_lexicalEnvironmentRegister->index());
     }
-    RegisterID* localArgumentsRegister = nullptr;
     RegisterID* scratch = addVar();
     m_symbolTable->setCaptureStart(virtualRegisterForLocal(m_codeBlock->m_numVars).offset());
 
     if (functionBody->usesArguments() || codeBlock->usesEval()) { // May reify arguments object.
         RegisterID* unmodifiedArgumentsRegister = addVar(); // Anonymous, so it can't be modified by user code.
         RegisterID* argumentsRegister = addVar(propertyNames().arguments, IsVariable, NotWatchable); // Can be changed by assigning to 'arguments'.
 
-        localArgumentsRegister = argumentsRegister;
-
         // We can save a little space by hard-coding the knowledge that the two
         // 'arguments' values are stored in consecutive registers, and storing
         // only the index of the assignable one.
@@ -279,15 +274,6 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionBodyNode* functionBody, Unl
                 initializeCapturedVariable(argumentsRegister, propertyNames().arguments, argumentsRegister);
                 RegisterID* uncheckedArgumentsRegister = &registerFor(JSC::unmodifiedArgumentsRegister(m_codeBlock->argumentsRegister()).offset());
                 initializeCapturedVariable(uncheckedArgumentsRegister, propertyNames().arguments, uncheckedArgumentsRegister);
-                if (functionBody->modifiesArguments()) {
-                    emitOpcode(op_mov);
-                    instructions().append(argumentsRegister->index());
-                    instructions().append(addConstantValue(jsUndefined())->index());
-                    emitOpcode(op_mov);
-                    instructions().append(uncheckedArgumentsRegister->index());
-                    instructions().append(addConstantValue(jsUndefined())->index());
-                    localArgumentsRegister = nullptr;
-                }
             }
         }
     }
@@ -400,7 +386,6 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionBodyNode* functionBody, Unl
     int nextParameterIndex = CallFrame::thisArgumentOffset();
     m_thisRegister.setIndex(nextParameterIndex++);
     m_codeBlock->addParameter();
-
     for (size_t i = 0; i < parameters.size(); ++i, ++nextParameterIndex) {
         int index = nextParameterIndex;
         auto pattern = parameters.at(i);
@@ -434,7 +419,6 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionBodyNode* functionBody, Unl
         instructions().append(0);
         instructions().append(0);
     }
-    m_localArgumentsRegister = localArgumentsRegister;
 }
 
 BytecodeGenerator::BytecodeGenerator(VM& vm, EvalNode* evalNode, UnlinkedEvalCodeBlock* codeBlock, DebuggerMode debuggerMode, ProfilerMode profilerMode)
@@ -447,7 +431,6 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, EvalNode* evalNode, UnlinkedEvalCod
     , m_lexicalEnvironmentRegister(0)
     , m_emptyValueRegister(0)
     , m_globalObjectRegister(0)
-    , m_localArgumentsRegister(0)
     , m_finallyDepth(0)
     , m_localScopeDepth(0)
     , m_codeType(EvalCode)
@@ -562,17 +545,19 @@ bool BytecodeGenerator::willResolveToArguments(const Identifier& ident)
     if (entry.isNull())
         return false;
 
-    if (m_codeBlock->usesArguments() && m_codeType == FunctionCode && m_localArgumentsRegister)
+    if (m_codeBlock->usesArguments() && m_codeType == FunctionCode)
         return true;
 
     return false;
 }
 
-RegisterID* BytecodeGenerator::uncheckedLocalArgumentsRegister()
+RegisterID* BytecodeGenerator::uncheckedRegisterForArguments()
 {
     ASSERT(willResolveToArguments(propertyNames().arguments));
-    ASSERT(m_localArgumentsRegister);
-    return m_localArgumentsRegister;
+
+    SymbolTableEntry entry = symbolTable().get(propertyNames().arguments.impl());
+    ASSERT(!entry.isNull());
+    return &registerFor(entry.getIndex());
 }
 
 RegisterID* BytecodeGenerator::createLazyRegisterIfNecessary(RegisterID* reg)
@@ -1843,7 +1828,7 @@ RegisterID* BytecodeGenerator::emitCall(OpcodeID opcodeID, RegisterID* dst, Regi
             auto expression = static_cast<SpreadExpressionNode*>(n->m_expr)->expression();
             RefPtr<RegisterID> argumentRegister;
             if (expression->isResolveNode() && willResolveToArguments(static_cast<ResolveNode*>(expression)->identifier()) && !symbolTable().slowArguments())
-                argumentRegister = uncheckedLocalArgumentsRegister();
+                argumentRegister = uncheckedRegisterForArguments();
             else
                 argumentRegister = expression->emitBytecode(*this, callArguments.argumentRegister(0));
             RefPtr<RegisterID> thisRegister = emitMove(newTemporary(), callArguments.thisRegister());
@@ -1985,7 +1970,7 @@ RegisterID* BytecodeGenerator::emitConstruct(RegisterID* dst, RegisterID* func,
             auto expression = static_cast<SpreadExpressionNode*>(n->m_expr)->expression();
             RefPtr<RegisterID> argumentRegister;
             if (expression->isResolveNode() && willResolveToArguments(static_cast<ResolveNode*>(expression)->identifier()) && !symbolTable().slowArguments())
-                argumentRegister = uncheckedLocalArgumentsRegister();
+                argumentRegister = uncheckedRegisterForArguments();
             else
                 argumentRegister = expression->emitBytecode(*this, callArguments.argumentRegister(0));
             return emitConstructVarargs(dst, func, argumentRegister.get(), newTemporary(), 0, callArguments.profileHookRegister(), divot, divotStart, divotEnd);
@@ -2553,13 +2538,13 @@ void BytecodeGenerator::emitEnumeration(ThrowableExpressionData* node, Expressio
         emitJump(loopCondition.get());
         emitLabel(loopStart.get());
         emitLoopHint();
-        emitGetArgumentByVal(value.get(), uncheckedLocalArgumentsRegister(), index.get());
+        emitGetArgumentByVal(value.get(), uncheckedRegisterForArguments(), index.get());
         callBack(*this, value.get());
 
         emitLabel(scope->continueTarget());
         emitInc(index.get());
         emitLabel(loopCondition.get());
-        RefPtr<RegisterID> length = emitGetArgumentsLength(newTemporary(), uncheckedLocalArgumentsRegister());
+        RefPtr<RegisterID> length = emitGetArgumentsLength(newTemporary(), uncheckedRegisterForArguments());
         emitJumpIfTrue(emitEqualityOp(op_less, newTemporary(), index.get(), length.get()), loopStart.get());
         emitLabel(scope->breakTarget());
         return;

Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

@@ -277,9 +277,7 @@ namespace JSC {
         void setIsNumericCompareFunction(bool isNumericCompareFunction);
 
         bool willResolveToArguments(const Identifier&);
-
-        bool hasSafeLocalArgumentsRegister() { return m_localArgumentsRegister; }
-        RegisterID* uncheckedLocalArgumentsRegister();
+        RegisterID* uncheckedRegisterForArguments();
 
         bool isCaptured(int operand);
         CaptureMode captureMode(int operand) { return isCaptured(operand) ? IsCaptured : NotCaptured; }
@@ -754,8 +752,6 @@ namespace JSC {
         RegisterID* m_lexicalEnvironmentRegister;
         RegisterID* m_emptyValueRegister;
         RegisterID* m_globalObjectRegister;
-        RegisterID* m_localArgumentsRegister;
-
         Vector<Identifier, 16> m_watchableVariables;
         SegmentedVector<RegisterID, 32> m_constantPoolRegisters;
         SegmentedVector<RegisterID, 32> m_calleeRegisters;

Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

@@ -386,7 +386,7 @@ RegisterID* BracketAccessorNode::emitBytecode(BytecodeGenerator& generator, Regi
         && !generator.symbolTable().slowArguments()) {
         RegisterID* property = generator.emitNode(m_subscript);
         generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
-        return generator.emitGetArgumentByVal(generator.finalDestination(dst), generator.uncheckedLocalArgumentsRegister(), property);
+        return generator.emitGetArgumentByVal(generator.finalDestination(dst), generator.uncheckedRegisterForArguments(), property);
     }
 
     RefPtr<RegisterID> base = generator.emitNodeForLeftHandSide(m_base, m_subscriptHasAssignments, m_subscript->isPure(generator));
@@ -412,7 +412,7 @@ RegisterID* DotAccessorNode::emitBytecode(BytecodeGenerator& generator, Register
         if (!generator.willResolveToArguments(resolveNode->identifier()))
             goto nonArgumentsPath;
         generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
-        return generator.emitGetArgumentsLength(generator.finalDestination(dst), generator.uncheckedLocalArgumentsRegister());
+        return generator.emitGetArgumentsLength(generator.finalDestination(dst), generator.uncheckedRegisterForArguments());
     }
 
 nonArgumentsPath:
@@ -593,7 +593,7 @@ static RegisterID* getArgumentByVal(BytecodeGenerator& generator, ExpressionNode
         && generator.willResolveToArguments(static_cast<ResolveNode*>(base)->identifier())
         && !generator.symbolTable().slowArguments()) {
         generator.emitExpressionInfo(divot, divotStart, divotEnd);
-        return generator.emitGetArgumentByVal(generator.finalDestination(dst), generator.uncheckedLocalArgumentsRegister(), property);
+        return generator.emitGetArgumentByVal(generator.finalDestination(dst), generator.uncheckedRegisterForArguments(), property);
     }
     return nullptr;
 }
@@ -621,7 +621,7 @@ RegisterID* CallFunctionCallDotNode::emitBytecode(BytecodeGenerator& generator,
             RefPtr<RegisterID> thisRegister = getArgumentByVal(generator, subject, generator.emitLoad(0, jsNumber(0)), 0, spread->divot(), spread->divotStart(), spread->divotEnd());
             RefPtr<RegisterID> argumentsRegister;
             if (thisRegister)
-                argumentsRegister = generator.uncheckedLocalArgumentsRegister();
+                argumentsRegister = generator.uncheckedRegisterForArguments();
             else {
                 argumentsRegister = generator.emitNode(subject);
                 generator.emitExpressionInfo(spread->divot(), spread->divotStart(), spread->divotEnd());
@@ -749,7 +749,7 @@ RegisterID* ApplyFunctionCallDotNode::emitBytecode(BytecodeGenerator& generator,
         RefPtr<RegisterID> argsRegister;
         ArgumentListNode* args = m_args->m_listNode->m_next;
         if (args->m_expr->isResolveNode() && generator.willResolveToArguments(static_cast<ResolveNode*>(args->m_expr)->identifier()) && !generator.symbolTable().slowArguments())
-            argsRegister = generator.uncheckedLocalArgumentsRegister();
+            argsRegister = generator.uncheckedRegisterForArguments();
         else
             argsRegister = generator.emitNode(args->m_expr);
 
@@ -2721,15 +2721,15 @@ RegisterID* ArrayPatternNode::emitDirectBinding(BytecodeGenerator& generator, Re
 {
     if (rhs->isResolveNode()
         && generator.willResolveToArguments(static_cast<ResolveNode*>(rhs)->identifier())
-        && generator.hasSafeLocalArgumentsRegister()&& !generator.symbolTable().slowArguments()) {
+        && !generator.symbolTable().slowArguments()) {
         for (size_t i = 0; i < m_targetPatterns.size(); i++) {
             auto target = m_targetPatterns[i];
             if (!target)
                 continue;
 
             RefPtr<RegisterID> temp = generator.newTemporary();
             generator.emitLoad(temp.get(), jsNumber(i));
-            generator.emitGetArgumentByVal(temp.get(), generator.uncheckedLocalArgumentsRegister(), temp.get());
+            generator.emitGetArgumentByVal(temp.get(), generator.uncheckedRegisterForArguments(), temp.get());
             target->bindValue(generator, temp.get());
         }
         if (dst == generator.ignoredResult() || !dst)

Source/JavaScriptCore/interpreter/StackVisitor.cpp

@@ -295,9 +295,6 @@ Arguments* StackVisitor::Frame::existingArguments()
     else
 #endif // ENABLE(DFG_JIT)
         reg = codeBlock()->argumentsRegister();
-
-    if (codeBlock()->needsActivation())
-        return jsCast<Arguments*>(callFrame()->lexicalEnvironment()->registerAt(unmodifiedArgumentsRegister(reg).offset()).get());
 
     JSValue result = callFrame()->r(unmodifiedArgumentsRegister(reg).offset()).jsValue();
     if (!result || !result.isCell()) // Protect against Undefined in case we throw in op_enter.

Source/JavaScriptCore/parser/Nodes.h

@@ -1440,7 +1440,6 @@ namespace JSC {
         bool usesEval() const { return m_features & EvalFeature; }
         bool usesArguments() const { return (m_features & ArgumentsFeature) && !(m_features & ShadowsArgumentsFeature); }
         bool modifiesParameter() const { return m_features & ModifiedParameterFeature; }
-        bool modifiesArguments() const { return m_features & (EvalFeature | ModifiedArgumentsFeature); }
         bool isStrictMode() const { return m_features & StrictModeFeature; }
         void setUsesArguments() { m_features |= ArgumentsFeature; }
         bool usesThis() const { return m_features & ThisFeature; }

Source/JavaScriptCore/parser/Parser.cpp

@@ -271,8 +271,7 @@ String Parser<LexerType>::parseInner()
 
     IdentifierSet capturedVariables;
     bool modifiedParameter = false;
-    bool modifiedArguments = false;
-    scope->getCapturedVariables(capturedVariables, modifiedParameter, modifiedArguments);
+    scope->getCapturedVariables(capturedVariables, modifiedParameter);
 
     CodeFeatures features = context.features();
     if (scope->strictMode())
@@ -281,8 +280,7 @@ String Parser<LexerType>::parseInner()
         features |= ShadowsArgumentsFeature;
     if (modifiedParameter)
         features |= ModifiedParameterFeature;
-    if (modifiedArguments)
-        features |= ModifiedArgumentsFeature;
+    
     Vector<RefPtr<StringImpl>> closedVariables;
     if (m_parsingBuiltin) {
         RELEASE_ASSERT(!capturedVariables.size());
@@ -294,10 +292,6 @@ String Parser<LexerType>::parseInner()
 
             if (scope->hasDeclaredParameter(Identifier(m_vm, variable.get())))
                 continue;
-
-            if (variable == m_vm->propertyNames->arguments.impl())
-                continue;
-
             closedVariables.append(variable);
         }
     }

Source/JavaScriptCore/parser/Parser.h

@@ -279,7 +279,7 @@ struct Scope {
         return true;
     }
 
-    void getCapturedVariables(IdentifierSet& capturedVariables, bool& modifiedParameter, bool& modifiedArguments)
+    void getCapturedVariables(IdentifierSet& capturedVariables, bool& modifiedParameter)
     {
         if (m_needsFullActivation || m_usesEval) {
             modifiedParameter = true;
@@ -292,13 +292,9 @@ struct Scope {
             capturedVariables.add(*ptr);
         }
         modifiedParameter = false;
-        if (shadowsArguments())
-            modifiedArguments = true;
         if (m_declaredParameters.size()) {
             IdentifierSet::iterator end = m_writtenVariables.end();
             for (IdentifierSet::iterator ptr = m_writtenVariables.begin(); ptr != end; ++ptr) {
-                if (*ptr == m_vm->propertyNames->arguments.impl())
-                    modifiedArguments = true;
                 if (!m_declaredParameters.contains(*ptr))
                     continue;
                 modifiedParameter = true;

Source/JavaScriptCore/parser/ParserModes.h

@@ -75,7 +75,6 @@ const CodeFeatures ThisFeature = 1 << 4;
 const CodeFeatures StrictModeFeature = 1 << 5;
 const CodeFeatures ShadowsArgumentsFeature = 1 << 6;
 const CodeFeatures ModifiedParameterFeature = 1 << 7;
-const CodeFeatures ModifiedArgumentsFeature = 1 << 8;
 
 const CodeFeatures AllFeatures = EvalFeature | ArgumentsFeature | WithFeature | CatchFeature | ThisFeature | StrictModeFeature | ShadowsArgumentsFeature | ModifiedParameterFeature;