[JSC] Optimize Number.prototype.toString on Int32 / Int52 / Double
Created attachment 299804 [details] Patch
Comment on attachment 299804 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299804&action=review > Source/JavaScriptCore/dfg/DFGOperations.h:162 > +char* JIT_OPERATION operationInt32ToString(ExecState*, int32_t, int32_t); Why not make these return JSCell*? > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:407 > + result = String::numberToStringECMAScript(value.asNumber()); Why not always call this? > Source/JavaScriptCore/runtime/StringConstructor.cpp:47 > + fromCharCode stringFromCharCode DontEnum|Function 1 FromCharCodeIntrinsic I think this should be a separate patch.
(In reply to comment #2) > Comment on attachment 299804 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=299804&action=review > > > Source/JavaScriptCore/dfg/DFGOperations.h:162 > > +char* JIT_OPERATION operationInt32ToString(ExecState*, int32_t, int32_t); > > Why not make these return JSCell*? > > > Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:407 > > + result = String::numberToStringECMAScript(value.asNumber()); > > Why not always call this? > > > Source/JavaScriptCore/runtime/StringConstructor.cpp:47 > > + fromCharCode stringFromCharCode DontEnum|Function 1 FromCharCodeIntrinsic > > I think this should be a separate patch. Also please fix 32-bit and windows build issues.
Comment on attachment 299804 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299804&action=review Thanks! >>> Source/JavaScriptCore/dfg/DFGOperations.h:162 >>> +char* JIT_OPERATION operationInt32ToString(ExecState*, int32_t, int32_t); >> >> Why not make these return JSCell*? > > Also please fix 32-bit and windows build issues. This is because the signatures of these functions are typedefed as P_ (returning pointer) to reuse the existing implementation of callOperation. I'll check Win32 build issue. >> Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:407 >> + result = String::numberToStringECMAScript(value.asNumber()); > > Why not always call this? I followed the style of the ValueAdd case. But maybe, calling numberToStringECMAScript is OK. I'll check that. >> Source/JavaScriptCore/runtime/StringConstructor.cpp:47 >> + fromCharCode stringFromCharCode DontEnum|Function 1 FromCharCodeIntrinsic > > I think this should be a separate patch. I've just uploaded this part in the separate patch :) https://bugs.webkit.org/show_bug.cgi?id=167505
(In reply to comment #4) > Comment on attachment 299804 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=299804&action=review > > Thanks! > > >>> Source/JavaScriptCore/dfg/DFGOperations.h:162 > >>> +char* JIT_OPERATION operationInt32ToString(ExecState*, int32_t, int32_t); > >> > >> Why not make these return JSCell*? > > > > Also please fix 32-bit and windows build issues. > > This is because the signatures of these functions are typedefed as P_ > (returning pointer) to reuse the existing implementation of callOperation. > I'll check Win32 build issue. > > >> Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:407 > >> + result = String::numberToStringECMAScript(value.asNumber()); > > > > Why not always call this? > > I followed the style of the ValueAdd case. But maybe, calling > numberToStringECMAScript is OK. > I'll check that. I think it's OK to be consistent with what Add does here. > > >> Source/JavaScriptCore/runtime/StringConstructor.cpp:47 > >> + fromCharCode stringFromCharCode DontEnum|Function 1 FromCharCodeIntrinsic > > > > I think this should be a separate patch. > > I've just uploaded this part in the separate patch :) > https://bugs.webkit.org/show_bug.cgi?id=167505
Oops, I forgot this patch! I'll land it with pointed fixes.
Created attachment 304995 [details] Patch Patch for landing
Committed r214219: <http://trac.webkit.org/changeset/214219>