170207 – AssemblyHelpers should not have a VM field

RESOLVED FIXED 170207

AssemblyHelpers should not have a VM field

https://bugs.webkit.org/show_bug.cgi?id=170207

Summary AssemblyHelpers should not have a VM field

Saam Barati

Reported 2017-03-28 15:10:01 PDT

APIs that need VM, should take one as a parameter. When doing PIC for wasm, we don't want to tie code generation to a VM.

Attachments
patch (149.98 KB, patch) 2017-03-28 15:44 PDT, Saam Barati	ysuzuki: review+	Details Formatted Diff Diff
patch for landing (152.40 KB, patch) 2017-03-28 16:12 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch for landing (152.61 KB, patch) 2017-03-28 16:30 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch for landing (152.85 KB, patch) 2017-03-28 16:39 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch for landing (152.74 KB, patch) 2017-03-28 16:50 PDT, Saam Barati	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews112 for mac-elcapitan (2.13 MB, application/zip) 2017-03-28 18:15 PDT, Build Bot	no flags	Details
patch for landing (156.30 KB, patch) 2017-03-28 20:18 PDT, Saam Barati	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Saam Barati

Comment 1 2017-03-28 15:44:56 PDT

Created attachment 305656 [details] patch

Saam Barati

Comment 2 2017-03-28 15:46:09 PDT

Comment on attachment 305656 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=305656&action=review > Source/JavaScriptCore/ChangeLog:9 > + independent code for Wasm, we can't want to tie code generation to a VM. will fix typo locally.

Build Bot

Comment 3 2017-03-28 15:46:52 PDT

Attachment 305656 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/AssemblyHelpers.h:1546: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 50 files If any of these errors are false positives, please file a bug against check-webkit-style.

Yusuke Suzuki

Comment 4 2017-03-28 15:56:46 PDT

Comment on attachment 305656 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=305656&action=review r=me with comments. > Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:-50 > - Why is this dropped? > Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp:-54 > - Ditto. > Source/JavaScriptCore/ftl/FTLThunks.cpp:169 > + AssemblyHelpers jit(0); Let's use nullptr. > Source/JavaScriptCore/jit/AssemblyHelpers.h:-977 > - void debugCall(V_DebugOperation_EPP function, void* argument) Why is this utility dropped? I think it is still useful (And I think moving it to cpp file is better).

Saam Barati

Comment 5 2017-03-28 16:01:13 PDT

Comment on attachment 305656 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=305656&action=review >> Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:-50 >> - > > Why is this dropped? I think we should just use probes.

Saam Barati

Comment 6 2017-03-28 16:04:21 PDT

Comment on attachment 305656 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=305656&action=review >>> Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:-50 >>> - >> >> Why is this dropped? > > I think we should just use probes. I'll just keep this utility function for now.

Saam Barati

Comment 7 2017-03-28 16:12:41 PDT

Created attachment 305660 [details] patch for landing If all platforms build.

Build Bot

Comment 8 2017-03-28 16:14:19 PDT

Attachment 305660 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/AssemblyHelpers.h:1550: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 50 files If any of these errors are false positives, please file a bug against check-webkit-style.

Saam Barati

Comment 9 2017-03-28 16:30:54 PDT

Created attachment 305664 [details] patch for landing

Build Bot

Comment 10 2017-03-28 16:34:11 PDT

Attachment 305664 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/AssemblyHelpers.h:1551: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 50 files If any of these errors are false positives, please file a bug against check-webkit-style.

Saam Barati

Comment 11 2017-03-28 16:39:48 PDT

Created attachment 305666 [details] patch for landing Trying to fix 32-bit build issues.

Build Bot

Comment 12 2017-03-28 16:42:29 PDT

Attachment 305666 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/AssemblyHelpers.h:1551: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 50 files If any of these errors are false positives, please file a bug against check-webkit-style.

Saam Barati

Comment 13 2017-03-28 16:50:19 PDT

Created attachment 305672 [details] patch for landing I think 32-bit builds now.

Build Bot

Comment 14 2017-03-28 16:54:26 PDT

Attachment 305672 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/AssemblyHelpers.h:1551: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 50 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 15 2017-03-28 18:15:14 PDT

Comment on attachment 305672 [details] patch for landing Attachment 305672 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3430388 Number of test failures exceeded the failure limit.

Build Bot

Comment 16 2017-03-28 18:15:17 PDT

Created attachment 305688 [details] Archive of layout-test-results from ews112 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Saam Barati

Comment 17 2017-03-28 20:18:45 PDT

Created attachment 305699 [details] patch for landing Fixed the crash. It was from accidentally capturing |this| inside a lambda in FTLLower

Build Bot

Comment 18 2017-03-28 20:21:54 PDT

Attachment 305699 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/AssemblyHelpers.h:1551: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 50 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Commit Bot

Comment 19 2017-03-28 23:15:27 PDT

Comment on attachment 305699 [details] patch for landing Clearing flags on attachment: 305699 Committed r214531: <http://trac.webkit.org/changeset/214531>

WebKit Commit Bot

Comment 20 2017-03-28 23:15:31 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component JavaScriptCore

Assignee

Saam Barati

Reported

2017-03-28 15:10 PDT

Modified

2017-03-28 23:15 PDT History

CC List

12 users Show

URL

Keywords

Depends on

Blocks

168264

Dependencies

tree graph