Bug 170207

Summary:

AssemblyHelpers should not have a VM field

Product:

WebKit

Reporter:

Saam Barati <saam>

Component:

JavaScriptCore

Assignee:

Saam Barati <saam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, buildbot, commit-queue, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, ticaiolima, ysuzuki

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

168264

Attachments:

Description	Flags
patch	ysuzuki: review+
patch for landing	none
patch for landing	none
patch for landing	none
patch for landing	buildbot: commit-queue-
Archive of layout-test-results from ews112 for mac-elcapitan	none
patch for landing	none

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.