Add LLInt fast path for less, lesseq, greater and greatereq
Created attachment 429753 [details] Patch
Created attachment 429929 [details] Patch
Created attachment 430035 [details] Patch
Created attachment 430038 [details] Patch
Created attachment 430047 [details] Patch
<rdar://problem/78761613>
Created attachment 430427 [details] Patch
Comment on attachment 430427 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430427&action=review > Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:2003 > + ci2ds t1, ft0 nit: no reason to convert t1 to a float before branching. > Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:2194 > + ci2ds t0, ft0 same nit here. I saw we do this in other places, so it's up to you if you want to move it.
Created attachment 430474 [details] Patch
Thanks for the review, Tadeu! I just sent a patch to see if ews is back and included your suggestion. Later I'll update the patch with some microbenchmarks I created.
Created attachment 430608 [details] Patch
Created attachment 430696 [details] Patch
Created attachment 430697 [details] Patch
Created attachment 430752 [details] Patch
Created attachment 430763 [details] Patch
Created attachment 430859 [details] Patch
On x86_64: baseline changes number-comparison-inline 12.2677+-0.0568 ^ 9.0738+-0.1066 ^ definitely 1.3520x faster <geometric> 12.2677+-0.0568 ^ 9.0738+-0.1066 ^ definitely 1.3520x faster On ARMv7: number-comparison-inline 58.4234+-1.0436 ^ 48.7810+-0.3148 ^ definitely 1.1977x faster <geometric> 58.4234+-1.0436 ^ 48.7810+-0.3148 ^ definitely 1.1977x faster
Comment on attachment 430427 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430427&action=review > Source/JavaScriptCore/ChangeLog:7 > + We are missing CL description for this patch. We should explain here the motivation why we are doing this change and explain changes a bit as well. In addition, It would quite interesting getting documented here the results for running at least JetStream 2, for LLInt-only (You can use `PerformanceTest/JetStream2/watch-cli.js` for it) and results with full JIT support.
Created attachment 431083 [details] Patch
Created attachment 431090 [details] Patch
JetStream2 results: x86_64 jit: a mean = 286.20032 b mean = 290.47749 pValue = 0.3039127831 (Bigger means are better.) 1.015 times better Results ARE NOT significant x86_64 no-jit: a mean = 38.51948 b mean = 39.22851 pValue = 0.0876327821 (Bigger means are better.) 1.018 times better Results ARE NOT significant ARMv7 no-jit: a mean = 10.33014 b mean = 10.29042 pValue = 0.2110898172 (Bigger means are better.) 1.004 times worse Results ARE NOT significant
Comment on attachment 431090 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431090&action=review LGTM as well. Since Tadeu gave you a r+ already, following patches you don’t need to set “r?" again, only if you made a major changes from previous patch. Ping me when you think patch is ready to get cq+. > Source/JavaScriptCore/ChangeLog:9 > + in LLInt, so we donât need to go to slow path for those cases. nit: there’s a strange character here. > Source/JavaScriptCore/ChangeLog:12 > + instruction for ARMv7, MIPS and CLoop. Could you include the numbers of benchmarks here? Also, it’s important to highlight that those numbers are from a LLInt-only configuration.
Created attachment 431440 [details] Patch
Created attachment 432260 [details] Patch
Comment on attachment 432260 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432260&action=review > Source/JavaScriptCore/ChangeLog:21 > + * ARMv7 no-jit: 1.004 times worse I think those results were neutral, right?
Yeah, 0.4% is probably noise.
Committed r279343 (239211@main): <https://commits.webkit.org/239211@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 432260 [details].