Bug 195181 - cloop.rb shift mask should depend on the word size being shifted.
Summary: cloop.rb shift mask should depend on the word size being shifted.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
: 195175 199684 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-02-28 12:29 PST by Mark Lam
Modified: 2019-07-17 05:58 PDT (History)
8 users (show)

See Also:


Attachments
proposed patch. (2.06 KB, patch)
2019-02-28 12:35 PST, Mark Lam
ysuzuki: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2019-02-28 12:29:41 PST
Previously, we're always masking the shift amount with 0x1f.  This only works for 32-bit words.  For 64-bit words, the mask should be 0x3f.  For intptr_t shifts, the mask depends on sizeof(uintptr_t).
Comment 1 Radar WebKit Bug Importer 2019-02-28 12:30:06 PST
<rdar://problem/48484164>
Comment 2 Mark Lam 2019-02-28 12:35:11 PST
Created attachment 363250 [details]
proposed patch.
Comment 3 Yusuke Suzuki 2019-02-28 12:45:10 PST
Comment on attachment 363250 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=363250&action=review

r=me

> Source/JavaScriptCore/offlineasm/cloop.rb:429
> +    shiftMask = "((sizeof(uintptr_t) == 8) ? 0x3f : 0x1f)" if type == :int || type == :uint

Discussed with Mark. The type ":int" and ":uint" actually means "intptr_t" and "uintptr_t". Then this is correct.
It is nice if we have either of
1. Add a comment about it here now, and rename ":int" => ":intPtr" or something like that in a subsequent patch.
2. Rename them in this patch too.
Comment 4 Mark Lam 2019-02-28 12:49:35 PST
Thanks for the review.  I added the FIXME.  Landed in r242215: <http://trac.webkit.org/r242215>.
Comment 5 Mark Lam 2019-02-28 12:51:16 PST
*** Bug 195175 has been marked as a duplicate of this bug. ***
Comment 6 Alberto Garcia 2019-07-17 05:58:56 PDT
*** Bug 199684 has been marked as a duplicate of this bug. ***