Bug 211029

Summary: [JSC] Add fast path for BigInt32 left-shift
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
saam: review+
Patch for landing
none
Patch for landing
none
Patch for landing none

Yusuke Suzuki
Reported 2020-04-25 13:53:58 PDT
[JSC] Add fast path for BigInt32 left-shift
Attachments
Patch (8.03 KB, patch)
2020-04-25 13:56 PDT, Yusuke Suzuki 		saam: review+
DetailsFormatted DiffDiff
Patch for landing (3.71 KB, patch)
2020-04-25 14:24 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch for landing (8.87 KB, patch)
2020-04-25 14:28 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch for landing (8.56 KB, patch)
2020-04-25 14:30 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2020-04-25 13:56:03 PDT
Created attachment 397581 [details] Patch
Saam Barati
Comment 2 2020-04-25 13:59:25 PDT
Comment on attachment 397581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397581&action=review > Source/JavaScriptCore/runtime/Operations.h:776 > + int64_t result64 = static_cast<int64_t>(leftInt32) << rightInt32; you need to check if this is a left shift, right?
Saam Barati
Comment 3 2020-04-25 14:01:54 PDT
Comment on attachment 397581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397581&action=review Nice. r=me >> Source/JavaScriptCore/runtime/Operations.h:776 >> + int64_t result64 = static_cast<int64_t>(leftInt32) << rightInt32; > > you need to check if this is a left shift, right? sorry, I missed the above !isLeft branch
Yusuke Suzuki
Comment 4 2020-04-25 14:18:53 PDT
Comment on attachment 397581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397581&action=review > Source/JavaScriptCore/runtime/Operations.h:765 > rightInt32 = -rightInt32; BTW, while this is completely separate issue from this, is this right? What happens if rightInt32 is INT32_MIN?
Yusuke Suzuki
Comment 5 2020-04-25 14:22:42 PDT
Comment on attachment 397581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397581&action=review > Source/JavaScriptCore/runtime/Operations.h:-770 > - return jsBigInt32(leftInt32 >> std::min(rightInt32, 31)); I thought this is right in terms of int32 spec, but it seems that BigInt is not using rounding. So this is wrong. Will just use std::min.
Yusuke Suzuki
Comment 6 2020-04-25 14:24:48 PDT
Created attachment 397582 [details] Patch for landing
Yusuke Suzuki
Comment 7 2020-04-25 14:28:07 PDT
Created attachment 397583 [details] Patch for landing
Yusuke Suzuki
Comment 8 2020-04-25 14:30:32 PDT
Created attachment 397584 [details] Patch for landing
Yusuke Suzuki
Comment 9 2020-04-25 16:56:46 PDT
Committed r260711: <https://trac.webkit.org/changeset/260711>
Radar WebKit Bug Importer
Comment 10 2020-04-25 16:57:16 PDT
<rdar://problem/62377699>
Note You need to log in before you can comment on or make changes to this bug.