[JSC] OpPow should have a "small int exponent" fast path at lower tiers
Created attachment 447395 [details] Patch
Comment on attachment 447395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447395&action=review > Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:1120 > + bigt t1, 1000, .slow Let's define 1000 as a constant in C++. And use `constexpr XXX` here. And in power implementation, we should use that constant too.
Created attachment 447406 [details] Patch
Created attachment 447418 [details] Patch
Created attachment 447425 [details] Patch
Comment on attachment 447425 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447425&action=review A couple of notes. > Source/JavaScriptCore/jit/JITArithmetic.cpp:496 > + addSlowCase(branch32(GreaterThan, rightRegs.payloadGPR(), TrustedImm32(maxExponentForIntegerMathPow))); I do wonder if maxExponentForIntegerMathPow should be 1024 instead of 1000. It hardly matters but it seems weird to use a base-10 threshold for bitshifting. > Source/JavaScriptCore/jit/JITArithmetic.cpp:-964 > -/* ------------------------------ END: OP_ADD, OP_SUB, OP_MUL, OP_POW ------------------------------ */ (Removing this since it's an END with no BEGIN and this file has not had an OP_POW.)
Comment on attachment 447425 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447425&action=review r=me > Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1341 > + addq numberTag, t0 What is the purpose of this addition? t0 is already 1, so just ci2ds is fine (it is not boxed double etc.). > JSTests/microbenchmarks/pow-double-int.js:7 > +for (let i = -50; i <= 50; i += 0.1) > + for (let j = 0; j <= 1000; j++) > + exponentiate(i, j); Can you add a test case getting Infinity, -Infinity, and NaN for double input?
(In reply to Yusuke Suzuki from comment #8) > > Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1341 > > + addq numberTag, t0 > > What is the purpose of this addition? t0 is already 1, so just ci2ds is fine > (it is not boxed double etc.). Thanks for pointing this out -- I assumed that I needed it in order to do a double calculation with an immediate, but apparently I was wrong. > > JSTests/microbenchmarks/pow-double-int.js:7 > > +for (let i = -50; i <= 50; i += 0.1) > > + for (let j = 0; j <= 1000; j++) > > + exponentiate(i, j); > > Can you add a test case getting Infinity, -Infinity, and NaN for double > input? I think this is covered by the following: stress/pow-nan-behaviors.js stress/pow-with-constants.js But let me know if we need more.
Created attachment 447541 [details] Patch for landing
Committed r287236 (245396@main): <https://commits.webkit.org/245396@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447541 [details].
<rdar://problem/86680069>