This will allow other optimizations.
Created attachment 265945 [details] patch
Comment on attachment 265945 [details] patch r=me
Comment on attachment 265945 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=265945&action=review > Source/JavaScriptCore/ChangeLog:10 > + it might be a constant value in the precense of inlining in the DFG. typo: precense ==> presence. > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:5353 > SpeculateCellOperand array(this, node->child1()); > - GPRReg arrayGPR = array.gpr(); > - > - GPRTemporary argumentsLength(this); > - GPRReg argumentsLengthGPR = argumentsLength.gpr(); > - > GPRTemporary argumentsStart(this); > + SpeculateStrictInt32Operand arrayLength(this, node->child2()); Do we really need a speculation (i.e. runtime check) for the int32-ness of the array length? Since this value can only be populated with the result of op_get_rest_length, doesn't that already ensure that it is always an int32? Would a DFG compile time assertion suffice here instead? Hmmm, for that matter, can the array operand above be anything but a JSArray since the bytecode generator only passes an array as an operand to op_copy_rest. Will we ever have a chance to invoke op_copy_rest on variable input? If not, would a DFG compile time assertion suffice here as well? > Source/JavaScriptCore/jit/JITOpcodes.cpp:1415 > + unsigned numParamsToSkip = currentInstruction[2].u.unsignedValue; > + load32(payloadFor(JSStack::ArgumentCount), regT0); > + sub32(TrustedImm32(1), regT0); > + Jump zeroLength = branch32(LessThanOrEqual, regT0, TrustedImm32(numParamsToSkip)); > + sub32(TrustedImm32(numParamsToSkip), regT0); Technically, you should use Imm32(numParamsToSkip) here instead of TrustedImm32(...). That is because the numParamsToSkip is defined by the user and can be any value. Hence, we want to blind that constant if necessary. In practice for non-malicious code, numParamsToSkip will tend to be small and not need blinding. But if you use Imm32(), it will take care of figuring that out for you.
Comment on attachment 265945 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=265945&action=review >> Source/JavaScriptCore/jit/JITOpcodes.cpp:1415 >> + sub32(TrustedImm32(numParamsToSkip), regT0); > > Technically, you should use Imm32(numParamsToSkip) here instead of TrustedImm32(...). That is because the numParamsToSkip is defined by the user and can be any value. Hence, we want to blind that constant if necessary. In practice for non-malicious code, numParamsToSkip will tend to be small and not need blinding. But if you use Imm32(), it will take care of figuring that out for you. Thanks for letting me know. I didn't know this. I'll make the change and also in the other places I do TrustedImm32 w.r.t rest parameters.
landed in: http://trac.webkit.org/changeset/192814