Bug 191092 - [JSC][LLInt] Compact LLInt ASM code by removing unnecessary instructions
Summary: [JSC][LLInt] Compact LLInt ASM code by removing unnecessary instructions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-30 16:42 PDT by Yusuke Suzuki
Modified: 2018-10-30 23:15 PDT (History)
7 users (show)

See Also:

Attachments
Patch (8.95 KB, patch)
2018-10-30 16:45 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (8.95 KB, patch)
2018-10-30 17:15 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (8.90 KB, patch)
2018-10-30 22:18 PDT, Yusuke Suzuki 		saam: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2018-10-30 16:42:44 PDT 
[JSC][LLInt] Compact LLInt ASM code by removing unnecessary instructions
Comment 1 Yusuke Suzuki 2018-10-30 16:45:32 PDT 
Created attachment 353433 [details]
Patch
Comment 2 Yusuke Suzuki 2018-10-30 17:15:07 PDT 
Created attachment 353444 [details]
Patch
Comment 3 Yusuke Suzuki 2018-10-30 22:18:36 PDT 
Created attachment 353463 [details]
Patch
Comment 4 Saam Barati 2018-10-30 22:26:31 PDT 
Comment on attachment 353444 [details]
Patch

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

r=me if you fix arm64

> Source/JavaScriptCore/offlineasm/arm64.rb:491
> +            emitARM64LShift([Immediate.new(nil, imm.to_s(2).size - 1), operands[1]], kind)

What does size return? Why not just log base 2?
Comment 5 Yusuke Suzuki 2018-10-30 23:11:52 PDT 
Comment on attachment 353444 [details]
Patch

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

>> Source/JavaScriptCore/offlineasm/arm64.rb:491
>> +            emitARM64LShift([Immediate.new(nil, imm.to_s(2).size - 1), operands[1]], kind)
> 
> What does size return? Why not just log base 2?

to_s(2) returns string with radix = 2. So, imm (4) .to_s(2) returns "100". "100".size - 1 => 2, which is the same to Math.log2(4).to_i.
Changed to `Math.log2(imm).to_i`.
Comment 6 Yusuke Suzuki 2018-10-30 23:14:09 PDT 
Committed r237627: <https://trac.webkit.org/changeset/237627>
Comment 7 Radar WebKit Bug Importer 2018-10-30 23:15:25 PDT 
<rdar://problem/45692456>