Changeset 102545 in webkit for trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

Timestamp:

Dec 11, 2011, 4:35:51 PM (13 years ago)

Author:

[email protected]

Message:

v8 benchmark takes 12-13 million function call slow paths due to extra arguments
​https://bugs.webkit.org/show_bug.cgi?id=74244

Reviewed by Filip Pizlo.

.arguments function of order the Reversed

10% speedup on v8-raytrace, 1.7% speedup on v8 overall, neutral on Kraken
and SunSpider.

• bytecode/CodeBlock.h:

(JSC::CodeBlock::valueProfileForArgument): Clarified that the interface
to this function is an argument number.

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::BytecodeGenerator):
(JSC::BytecodeGenerator::emitCall):
(JSC::BytecodeGenerator::emitConstruct):
(JSC::BytecodeGenerator::isArgumentNumber): Switched to using CallFrame
helper functions for computing offsets for arguments, rather than doing
the math by hand.

Switched to iterating argument offsets backwards (--) instead of forwards (++).

• bytecompiler/BytecodeGenerator.h:

(JSC::CallArguments::thisRegister):
(JSC::CallArguments::argumentRegister):
(JSC::CallArguments::registerOffset): Updated for arguments being reversed.

• bytecompiler/NodesCodegen.cpp: Allocate arguments in reverse order.

• dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::getArgument):
(JSC::DFG::ByteCodeParser::setArgument):
(JSC::DFG::ByteCodeParser::flush):
(JSC::DFG::ByteCodeParser::addCall):
(JSC::DFG::ByteCodeParser::handleCall):
(JSC::DFG::ByteCodeParser::handleInlining):
(JSC::DFG::ByteCodeParser::handleMinMax):
(JSC::DFG::ByteCodeParser::handleIntrinsic):
(JSC::DFG::ByteCodeParser::parseBlock):
(JSC::DFG::ByteCodeParser::processPhiStack): Use abstract argument indices
that just-in-time convert to bytecode operands (i.e., indexes in the register
file) through helper functions. This means only one piece of code needs
to know how arguments are laid out in the register file.

• dfg/DFGGraph.cpp:

(JSC::DFG::Graph::dump): Ditto.

• dfg/DFGGraph.h:

(JSC::DFG::Graph::valueProfileFor): Ditto.

• dfg/DFGJITCompiler.cpp:

(JSC::DFG::JITCompiler::compileFunction): The whole point of this patch:
Treat too many arguments as an arity match.

• dfg/DFGOSRExit.h:

(JSC::DFG::OSRExit::variableForIndex):
(JSC::DFG::OSRExit::operandForIndex): Use helper functions, as above.

• dfg/DFGOperands.h:

(JSC::DFG::operandToArgument):
(JSC::DFG::argumentToOperand): These are now the only two lines of code in
the DFG compiler that know how arguments are laid out in memory.

(JSC::DFG::Operands::operand):
(JSC::DFG::Operands::setOperand): Use helper functions, as above.

• dfg/DFGOperations.cpp: The whole point of this patch:

Treat too many arguments as an arity match.

• dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::emitCall): Use helper functions, as above.

Also, don't tag the caller frame slot as a cell, because it's not a cell.

• dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::emitCall): Use helper functions, as above.

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compile): Use helper functions, as above.

(JSC::DFG::SpeculativeJIT::checkArgumentTypes): Use already-computed
argument virtual register instead of recomputing by hand.

• dfg/DFGSpeculativeJIT.h:

(JSC::DFG::SpeculativeJIT::callFrameSlot):
(JSC::DFG::SpeculativeJIT::argumentSlot):
(JSC::DFG::SpeculativeJIT::callFrameTagSlot):
(JSC::DFG::SpeculativeJIT::callFramePayloadSlot):
(JSC::DFG::SpeculativeJIT::argumentTagSlot):
(JSC::DFG::SpeculativeJIT::argumentPayloadSlot): Added a few helper
functions for dealing with callee arguments specifically. These still
build on top of our other helper functions, and have no direct knowledge
of how arguments are laid out in the register file.

(JSC::DFG::SpeculativeJIT::resetCallArguments):
(JSC::DFG::SpeculativeJIT::addCallArgument): Renamed argumentIndex to
argumentOffset to match CallFrame naming.

(JSC::DFG::SpeculativeJIT::valueSourceReferenceForOperand): Use helper
functions, as above.

• interpreter/CallFrame.h:

(JSC::ExecState::argumentOffset):
(JSC::ExecState::argumentOffsetIncludingThis):
(JSC::ExecState::argument):
(JSC::ExecState::setArgument):
(JSC::ExecState::thisArgumentOffset):
(JSC::ExecState::thisValue):
(JSC::ExecState::setThisValue):
(JSC::ExecState::offsetFor):
(JSC::ExecState::hostThisRegister):
(JSC::ExecState::hostThisValue): Added a bunch of helper functions for
computing where an argument is in the register file. Anything in the
runtime that needs to access arguments should use these helpers.

• interpreter/CallFrameClosure.h:

(JSC::CallFrameClosure::setThis):
(JSC::CallFrameClosure::setArgument):
(JSC::CallFrameClosure::resetCallFrame): This stuff is a lot simpler, now
that too many arguments counts as an arity match and doesn't require
preserving two copies of our arguments.

• interpreter/Interpreter.cpp:

(JSC::Interpreter::slideRegisterWindowForCall): Only need to do something
special if the caller provided too few arguments.

Key simplification: We never need to maintain two copies of our arguments
anymore.

(JSC::eval):
(JSC::loadVarargs): Use helper functions.

(JSC::Interpreter::unwindCallFrame): Updated for new interface.

(JSC::Interpreter::execute):
(JSC::Interpreter::executeCall):
(JSC::Interpreter::executeConstruct):
(JSC::Interpreter::prepareForRepeatCall): Seriously, though: use helper
functions.

(JSC::Interpreter::privateExecute): No need to check for stack overflow
when calling host functions because they have zero callee registers.

(JSC::Interpreter::retrieveArguments): Explicitly tear off the arguments
object, since there's no special constructor for this anymore.

• interpreter/Interpreter.h: Reduced the C++ re-entry depth because some

workers tests were hitting stack overflow in some of my testing. We should
make this test more exact in future.

• interpreter/RegisterFile.h: Death to all runtime knowledge of argument

location that does not belong to the CallFrame class!

• jit/JIT.cpp:

(JSC::JIT::privateCompile): I am a broken record and I use helper functions.

Also, the whole point of this patch: Treat too many arguments as an arity match.

• jit/JITCall32_64.cpp:

(JSC::JIT::compileLoadVarargs):

• jit/JITCall.cpp:

(JSC::JIT::compileLoadVarargs): Updated the argument copying math to use
helper functions, for backwards-correctness. Removed the condition
pertaining to declared argument count because, now that arguments are
always in just one place, this optimization is valid for all functions.
Standardized the if predicate for each line of the optimization. This might
fix a bug, but I couldn't get the bug to crash in practice.

• jit/JITOpcodes32_64.cpp:

(JSC::JIT::emit_op_create_arguments):
(JSC::JIT::emit_op_get_argument_by_val):
(JSC::JIT::emitSlow_op_get_argument_by_val):

• jit/JITOpcodes.cpp:

(JSC::JIT::emit_op_create_arguments):
(JSC::JIT::emit_op_get_argument_by_val):
(JSC::JIT::emitSlow_op_get_argument_by_val): Removed cti_op_create_arguments_no_params
optimization because it's no longer an optimization, now that arguments
are always contiguous in a known location.

Updated argument access opcode math for backwards-correctness.

• jit/JITStubs.cpp:

(JSC::arityCheckFor): Updated just like slideRegisterWindowForCall. This
function is slightly different because it copies the call frame in
addition to the arguments. (In the Interpreter, the call frame is not
set up by this point.)

(JSC::lazyLinkFor): The whole point of this patch: Treat too many
arguments as an arity match.

(JSC::DEFINE_STUB_FUNCTION): Updated for new iterface to tearOff().

• jit/JITStubs.h:
• jit/SpecializedThunkJIT.h:

(JSC::SpecializedThunkJIT::loadDoubleArgument):
(JSC::SpecializedThunkJIT::loadCellArgument):
(JSC::SpecializedThunkJIT::loadInt32Argument): Use helper functions! They
build strong bones and teeth!

• runtime/ArgList.cpp:

(JSC::ArgList::getSlice):
(JSC::MarkedArgumentBuffer::slowAppend):

• runtime/ArgList.h:

(JSC::MarkedArgumentBuffer::MarkedArgumentBuffer):
(JSC::MarkedArgumentBuffer::~MarkedArgumentBuffer):
(JSC::MarkedArgumentBuffer::at):
(JSC::MarkedArgumentBuffer::clear):
(JSC::MarkedArgumentBuffer::append):
(JSC::MarkedArgumentBuffer::removeLast):
(JSC::MarkedArgumentBuffer::last):
(JSC::ArgList::ArgList):
(JSC::ArgList::at): Updated for backwards-correctness. WTF::Vector doesn't
play nice with backwards-ness, so I changed to using manual allocation.

Fixed a FIXME about not all values being marked in the case of out-of-line
arguments. I had to rewrite the loop anyway, and I didn't feel like
maintaining fidelity to its old bugs.

• runtime/Arguments.cpp:

(JSC::Arguments::visitChildren):
(JSC::Arguments::copyToArguments):
(JSC::Arguments::fillArgList):
(JSC::Arguments::getOwnPropertySlotByIndex):
(JSC::Arguments::getOwnPropertySlot):
(JSC::Arguments::getOwnPropertyDescriptor):
(JSC::Arguments::putByIndex):
(JSC::Arguments::put):
(JSC::Arguments::tearOff):

• runtime/Arguments.h:

(JSC::Arguments::create):
(JSC::Arguments::Arguments):
(JSC::Arguments::argument):
(JSC::Arguments::finishCreation): Secondary benefit of this patch: deleted
lots of tricky code designed to maintain two different copies of function
arguments. Now that arguments are always contiguous in one place in memory,
this complexity can go away.

Reduced down to one create function for the Arguments class, from three.

Moved tearOff() into an out-of-line function because it's huge.

Moved logic about whether to tear off eagerly into the Arguments class,
so we didn't have to duplicate it elsewhere.

• runtime/JSActivation.cpp:

(JSC::JSActivation::JSActivation):
(JSC::JSActivation::visitChildren): Renamed m_numParametersMinusThis to
m_numCapturedArgs because if the value really were m_numParametersMinusThis
we would be marking too much. (We shouldn't mark 'this' because it can't
be captured.) Also, use helper functions.

• runtime/JSActivation.h:

(JSC::JSActivation::tearOff): Use helper functions.

• runtime/JSArray.cpp:

(JSC::JSArray::copyToArguments):

• runtime/JSArray.h: Use helper functions, as above.

File:

• trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (modified) (22 diffs)

trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

@@ -77 +77 @@
 
     // Helper for min and max.
-    bool handleMinMax(bool usesResult, int resultOperand, NodeType op, int firstArg, int lastArg);
+    bool handleMinMax(bool usesResult, int resultOperand, NodeType op, int registerOffset, int argumentCountIncludingThis);
     
     // Handle calls. This resolves issues surrounding inlining and intrinsics.
     void handleCall(Interpreter*, Instruction* currentInstruction, NodeType op, CodeSpecializationKind);
     // Handle inlining. Return true if it succeeded, false if we need to plant a call.
-    bool handleInlining(bool usesResult, int callTarget, NodeIndex callTargetNodeIndex, int resultOperand, bool certainAboutExpectedFunction, JSFunction*, int firstArg, int lastArg, unsigned nextOffset, CodeSpecializationKind);
+    bool handleInlining(bool usesResult, int callTarget, NodeIndex callTargetNodeIndex, int resultOperand, bool certainAboutExpectedFunction, JSFunction*, int registerOffset, int argumentCountIncludingThis, unsigned nextOffset, CodeSpecializationKind);
     // Handle intrinsic functions. Return true if it succeeded, false if we need to plant a call.
-    bool handleIntrinsic(bool usesResult, int resultOperand, Intrinsic, int firstArg, int lastArg, PredictedType prediction);
+    bool handleIntrinsic(bool usesResult, int resultOperand, Intrinsic, int registerOffset, int argumentCountIncludingThis, PredictedType prediction);
     // Prepare to parse a block.
     void prepareToParseBlock();
@@ -200 +200 @@
     NodeIndex getArgument(unsigned operand)
     {
-        unsigned argument = operand + m_codeBlock->m_numParameters + RegisterFile::CallFrameHeaderSize;
+        unsigned argument = operandToArgument(operand);
         ASSERT(argument < m_numArguments);
 
@@ -249 +249 @@
     void setArgument(int operand, NodeIndex value)
     {
-        unsigned argument = operand + m_codeBlock->m_numParameters + RegisterFile::CallFrameHeaderSize;
+        unsigned argument = operandToArgument(operand);
         ASSERT(argument < m_numArguments);
 
@@ -267 +267 @@
         int index;
         if (operandIsArgument(operand)) {
-            index = operand + m_codeBlock->m_numParameters + RegisterFile::CallFrameHeaderSize;
+            index = operandToArgument(operand);
             nodeIndex = m_currentBlock->variablesAtTail.argument(index);
         } else {
@@ -579 +579 @@
         addVarArgChild(get(currentInstruction[1].u.operand));
         int argCount = currentInstruction[2].u.operand;
+        if (RegisterFile::CallFrameHeaderSize + (unsigned)argCount > m_parameterSlots)
+            m_parameterSlots = RegisterFile::CallFrameHeaderSize + argCount;
+
         int registerOffset = currentInstruction[3].u.operand;
-        int firstArg = registerOffset - argCount - RegisterFile::CallFrameHeaderSize;
-        for (int argIdx = firstArg + (op == Construct ? 1 : 0); argIdx < firstArg + argCount; argIdx++)
-            addVarArgChild(get(argIdx));
+        int dummyThisArgument = op == Call ? 0 : 1;
+        for (int i = 0 + dummyThisArgument; i < argCount; ++i)
+            addVarArgChild(get(registerOffset + argumentToOperand(i)));
+
         NodeIndex call = addToGraph(Node::VarArg, op, OpInfo(0), OpInfo(prediction));
         if (interpreter->getOpcodeID(putInstruction->u.opcode) == op_call_put_result)
             set(putInstruction[1].u.operand, call);
-        if (RegisterFile::CallFrameHeaderSize + (unsigned)argCount > m_parameterSlots)
-            m_parameterSlots = RegisterFile::CallFrameHeaderSize + argCount;
         return call;
     }
@@ -896 +898 @@
         callType = UnknownFunction;
     if (callType != UnknownFunction) {
-        int argCount = currentInstruction[2].u.operand;
+        int argumentCountIncludingThis = currentInstruction[2].u.operand;
         int registerOffset = currentInstruction[3].u.operand;
-        int firstArg = registerOffset - argCount - RegisterFile::CallFrameHeaderSize;
-        int lastArg = firstArg + argCount - 1;
-                
+
         // Do we have a result?
         bool usesResult = false;
@@ -931 +931 @@
                 addToGraph(CheckFunction, OpInfo(expectedFunction), callTarget);
             
-            if (handleIntrinsic(usesResult, resultOperand, intrinsic, firstArg, lastArg, prediction)) {
+            if (handleIntrinsic(usesResult, resultOperand, intrinsic, registerOffset, argumentCountIncludingThis, prediction)) {
                 if (!certainAboutExpectedFunction) {
                     // Need to keep the call target alive for OSR. We could easily optimize this out if we wanted
@@ -941 +941 @@
                 return;
             }
-        } else if (handleInlining(usesResult, currentInstruction[1].u.operand, callTarget, resultOperand, certainAboutExpectedFunction, expectedFunction, firstArg, lastArg, nextOffset, kind))
+        } else if (handleInlining(usesResult, currentInstruction[1].u.operand, callTarget, resultOperand, certainAboutExpectedFunction, expectedFunction, registerOffset, argumentCountIncludingThis, nextOffset, kind))
             return;
     }
@@ -948 +948 @@
 }
 
-bool ByteCodeParser::handleInlining(bool usesResult, int callTarget, NodeIndex callTargetNodeIndex, int resultOperand, bool certainAboutExpectedFunction, JSFunction* expectedFunction, int firstArg, int lastArg, unsigned nextOffset, CodeSpecializationKind kind)
+bool ByteCodeParser::handleInlining(bool usesResult, int callTarget, NodeIndex callTargetNodeIndex, int resultOperand, bool certainAboutExpectedFunction, JSFunction* expectedFunction, int registerOffset, int argumentCountIncludingThis, unsigned nextOffset, CodeSpecializationKind kind)
 {
     // First, the really simple checks: do we have an actual JS function?
@@ -960 +960 @@
     // Does the number of arguments we're passing match the arity of the target? We could
     // inline arity check failures, but for simplicity we currently don't.
-    if (static_cast<int>(executable->parameterCount()) + 1 != lastArg - firstArg + 1)
+    if (static_cast<int>(executable->parameterCount()) + 1 != argumentCountIncludingThis)
         return false;
     
@@ -1001 +1001 @@
     // FIXME: Don't flush constants!
     
-    for (int arg = firstArg + 1; arg <= lastArg; ++arg)
-        flush(arg);
-    
-    int inlineCallFrameStart = m_inlineStackTop->remapOperand(lastArg) + 1;
+    for (int i = 1; i < argumentCountIncludingThis; ++i)
+        flush(registerOffset + argumentToOperand(i));
+    
+    int inlineCallFrameStart = m_inlineStackTop->remapOperand(registerOffset) - RegisterFile::CallFrameHeaderSize;
     
     // Make sure that the area used by the call frame is reserved.
@@ -1124 +1124 @@
 }
 
-bool ByteCodeParser::handleMinMax(bool usesResult, int resultOperand, NodeType op, int firstArg, int lastArg)
+bool ByteCodeParser::handleMinMax(bool usesResult, int resultOperand, NodeType op, int registerOffset, int argumentCountIncludingThis)
 {
     if (!usesResult)
         return true;
 
-    if (lastArg == firstArg) {
+    if (argumentCountIncludingThis == 1) { // Math.min()
         set(resultOperand, constantNaN());
         return true;
     }
      
-    if (lastArg == firstArg + 1) {
-        set(resultOperand, getToNumber(firstArg + 1));
+    if (argumentCountIncludingThis == 2) { // Math.min(x)
+        set(resultOperand, getToNumber(registerOffset + argumentToOperand(1)));
         return true;
     }
     
-    if (lastArg == firstArg + 2) {
-        set(resultOperand, addToGraph(op, OpInfo(NodeUseBottom), getToNumber(firstArg + 1), getToNumber(firstArg + 2)));
+    if (argumentCountIncludingThis == 3) { // Math.min(x, y)
+        set(resultOperand, addToGraph(op, OpInfo(NodeUseBottom), getToNumber(registerOffset + argumentToOperand(1)), getToNumber(registerOffset + argumentToOperand(2))));
         return true;
     }
@@ -1148 +1148 @@
 }
 
-bool ByteCodeParser::handleIntrinsic(bool usesResult, int resultOperand, Intrinsic intrinsic, int firstArg, int lastArg, PredictedType prediction)
+// FIXME: We dead-code-eliminate unused Math intrinsics, but that's invalid because
+// they need to perform the ToNumber conversion, which can have side-effects.
+bool ByteCodeParser::handleIntrinsic(bool usesResult, int resultOperand, Intrinsic intrinsic, int registerOffset, int argumentCountIncludingThis, PredictedType prediction)
 {
     switch (intrinsic) {
@@ -1158 +1160 @@
         }
         
-        // We don't care about the this argument. If we don't have a first
-        // argument then make this JSConstant(NaN).
-        int absArg = firstArg + 1;
-        if (absArg > lastArg) {
+        if (argumentCountIncludingThis == 1) { // Math.abs()
             set(resultOperand, constantNaN());
             return true;
@@ -1169 +1168 @@
             return false;
 
-        set(resultOperand, addToGraph(ArithAbs, OpInfo(NodeUseBottom), getToNumber(absArg)));
+        set(resultOperand, addToGraph(ArithAbs, OpInfo(NodeUseBottom), getToNumber(registerOffset + argumentToOperand(1))));
         return true;
     }
-        
+
     case MinIntrinsic:
-        return handleMinMax(usesResult, resultOperand, ArithMin, firstArg, lastArg);
+        return handleMinMax(usesResult, resultOperand, ArithMin, registerOffset, argumentCountIncludingThis);
         
     case MaxIntrinsic:
-        return handleMinMax(usesResult, resultOperand, ArithMax, firstArg, lastArg);
+        return handleMinMax(usesResult, resultOperand, ArithMax, registerOffset, argumentCountIncludingThis);
         
     case SqrtIntrinsic: {
@@ -1183 +1182 @@
             return true;
         
-        if (firstArg == lastArg) {
+        if (argumentCountIncludingThis == 1) { // Math.sqrt()
             set(resultOperand, constantNaN());
             return true;
@@ -1191 +1190 @@
             return false;
         
-        set(resultOperand, addToGraph(ArithSqrt, getToNumber(firstArg + 1)));
+        set(resultOperand, addToGraph(ArithSqrt, getToNumber(registerOffset + argumentToOperand(1))));
         return true;
     }
         
     case ArrayPushIntrinsic: {
-        if (firstArg + 1 != lastArg)
+        if (argumentCountIncludingThis != 2)
             return false;
         
-        NodeIndex arrayPush = addToGraph(ArrayPush, OpInfo(0), OpInfo(prediction), get(firstArg), get(firstArg + 1));
+        NodeIndex arrayPush = addToGraph(ArrayPush, OpInfo(0), OpInfo(prediction), get(registerOffset + argumentToOperand(0)), get(registerOffset + argumentToOperand(1)));
         if (usesResult)
             set(resultOperand, arrayPush);
@@ -1207 +1206 @@
         
     case ArrayPopIntrinsic: {
-        if (firstArg != lastArg)
+        if (argumentCountIncludingThis != 1)
             return false;
         
-        NodeIndex arrayPop = addToGraph(ArrayPop, OpInfo(0), OpInfo(prediction), get(firstArg));
+        NodeIndex arrayPop = addToGraph(ArrayPop, OpInfo(0), OpInfo(prediction), get(registerOffset + argumentToOperand(0)));
         if (usesResult)
             set(resultOperand, arrayPop);
@@ -1217 +1216 @@
 
     case CharCodeAtIntrinsic: {
-        if (firstArg + 1 != lastArg)
+        if (argumentCountIncludingThis != 2)
             return false;
-        if (!(m_graph[get(firstArg)].prediction() & PredictString))
+
+        int thisOperand = registerOffset + argumentToOperand(0);
+        if (!(m_graph[get(thisOperand)].prediction() & PredictString))
             return false;
         
-        NodeIndex storage = addToGraph(GetIndexedPropertyStorage, get(firstArg), getToInt32(firstArg + 1));
-        NodeIndex charCode = addToGraph(StringCharCodeAt, get(firstArg), getToInt32(firstArg + 1), storage);
+        int indexOperand = registerOffset + argumentToOperand(1);
+        NodeIndex storage = addToGraph(GetIndexedPropertyStorage, get(thisOperand), getToInt32(indexOperand));
+        NodeIndex charCode = addToGraph(StringCharCodeAt, get(thisOperand), getToInt32(indexOperand), storage);
+
         if (usesResult)
             set(resultOperand, charCode);
@@ -1230 +1233 @@
 
     case CharAtIntrinsic: {
-        if (firstArg + 1 != lastArg)
+        if (argumentCountIncludingThis != 2)
             return false;
-        if (!(m_graph[get(firstArg)].prediction() & PredictString))
+
+        int thisOperand = registerOffset + argumentToOperand(0);
+        if (!(m_graph[get(thisOperand)].prediction() & PredictString))
             return false;
 
-        NodeIndex storage = addToGraph(GetIndexedPropertyStorage, get(firstArg), getToInt32(firstArg + 1));
-        NodeIndex charCode = addToGraph(StringCharAt, get(firstArg), getToInt32(firstArg + 1), storage);
+        int indexOperand = registerOffset + argumentToOperand(1);
+        NodeIndex storage = addToGraph(GetIndexedPropertyStorage, get(thisOperand), getToInt32(indexOperand));
+        NodeIndex charCode = addToGraph(StringCharAt, get(thisOperand), getToInt32(indexOperand), storage);
+
         if (usesResult)
             set(resultOperand, charCode);
@@ -1268 +1275 @@
         m_graph.m_arguments.resize(m_numArguments);
         for (unsigned argument = 0; argument < m_numArguments; ++argument) {
-            NodeIndex setArgument = addToGraph(SetArgument, OpInfo(newVariableAccessData(argument - m_codeBlock->m_numParameters - RegisterFile::CallFrameHeaderSize)));
+            NodeIndex setArgument = addToGraph(SetArgument, OpInfo(newVariableAccessData(argumentToOperand(argument))));
             m_graph.m_arguments[argument] = setArgument;
             m_currentBlock->variablesAtHead.setArgumentFirstTime(argument, setArgument);
@@ -2208 +2215 @@
 #endif
 
-                valueInPredecessor = addToGraph(Phi, OpInfo(newVariableAccessData(stackType == ArgumentPhiStack ? varNo - m_codeBlock->m_numParameters - RegisterFile::CallFrameHeaderSize : varNo)));
+                valueInPredecessor = addToGraph(Phi, OpInfo(newVariableAccessData(stackType == ArgumentPhiStack ? argumentToOperand(varNo) : static_cast<int>(varNo))));
                 var = valueInPredecessor;
                 if (stackType == ArgumentPhiStack)