Bug 157931 - LLInt64 should have typed array fast paths for get_by_val
Summary: LLInt64 should have typed array fast paths for get_by_val
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 157922
  Show dependency tree / graph
 
Reported: 2016-05-19 16:31 PDT by Filip Pizlo
Modified: 2016-05-25 11:59 PDT (History)
14 users (show)

See Also:

Attachments
starting to do things (5.87 KB, patch)
2016-05-19 16:32 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (12.75 KB, patch)
2016-05-24 09:33 PDT, Filip Pizlo 		keith_miller: 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 Filip Pizlo 2016-05-19 16:31:32 PDT 
Patch forthcoming.
Comment 1 Filip Pizlo 2016-05-19 16:32:14 PDT 
Created attachment 279445 [details]
starting to do things
Comment 2 Filip Pizlo 2016-05-24 09:33:26 PDT 
Created attachment 279661 [details]
the patch

It works!
Comment 3 WebKit Commit Bot 2016-05-24 09:35:22 PDT 
Attachment 279661 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:59:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Filip Pizlo 2016-05-24 09:35:24 PDT 
Comment on attachment 279661 [details]
the patch

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

> Source/JavaScriptCore/offlineasm/backends.rb:117
> +            $stderr.puts "Caught it here"

I will remove this line.
Comment 5 Keith Miller 2016-05-24 09:48:09 PDT 
Comment on attachment 279661 [details]
the patch

r=me
Comment 6 Filip Pizlo 2016-05-24 10:08:03 PDT 
Landed in http://trac.webkit.org/changeset/201335
Comment 7 Csaba Osztrogonác 2016-05-24 12:44:18 PDT 
(In reply to comment #6)
> Landed in http://trac.webkit.org/changeset/201335

It broke the WinCairo build, see build.webkit.org foe details. But these logs aren't so verbose. cc-ing port maintainers too
Comment 8 Alex Christensen 2016-05-24 23:28:17 PDT 
Comment on attachment 279661 [details]
the patch

That bot is the only public 64-bit Windows bot, so this may have broken all of the Win64 build.
Comment 9 Alex Christensen 2016-05-25 10:50:48 PDT 
Commenting these two lines out of the x86_64 LowLevelInterpreterWin.asm makes it assemble successfully.  It doesn't like the loadbs and loadhs

movsbl byte ptr [0 + r9 + rdx * 1], eax                  ; C:/Users/Alex/Documents/WinCairoBot/win-cairo-release/build/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1504
...
movswl word ptr [0 + r9 + rdx * 2], eax                  ; C:/Users/Alex/Documents/WinCairoBot/win-cairo-release/build/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1508
Comment 10 Alex Christensen 2016-05-25 11:59:47 PDT 
Fixed in https://bugs.webkit.org/show_bug.cgi?id=158078