Bug 171088

Summary:

Update the MASM probe to only take 1 arg instead of 2 (in addition to the callback function).

Product:

WebKit

Reporter:

Mark Lam <mark.lam>

Component:

JavaScriptCore

Assignee:

Mark Lam <mark.lam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

fpizlo, jfbastien, keith_miller, msaboff, ryanhaddad, saam

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
proposed patch.	msaboff: review+

Mark Lam

Reported 2017-04-20 16:03:46 PDT

Experience shows that we never use the 2nd arg. So, let's remove it to reduce the footprint at each probe site. Also fix the MacroAssembler::print() function so that it is a no-op when !ENABLE(MASM_PROBE). This will allow us to have print() statements in JIT code without a lot of #if ENABLE(MASM_PROBE)s later.

Attachments
proposed patch. (36.56 KB, patch) 2017-04-20 16:43 PDT, Mark Lam	msaboff: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Mark Lam

Comment 1 2017-04-20 16:43:49 PDT

Created attachment 307660 [details] proposed patch.

Mark Lam

Comment 2 2017-04-20 16:45:30 PDT

I've done builds for ARMv7, ARM64, x86, and x86_64 with this patch, and ran a smoke test of printing something using the MASM print. I also did a build with !ENABLE(MASM_PROBE) to confirm that MASM print becomes a no-op in that case.

Michael Saboff

Comment 3 2017-04-20 16:51:37 PDT

Comment on attachment 307660 [details] proposed patch. r=me

Saam Barati

Comment 4 2017-04-20 16:52:05 PDT

r=me too

Mark Lam

Comment 5 2017-04-20 16:57:02 PDT

Thanks for the reviews. Landed in r215592: <http://trac.webkit.org/r215592>.

Ryan Haddad

Comment 6 2017-04-20 17:11:31 PDT

(In reply to Mark Lam from comment #5) > Thanks for the reviews. Landed in r215592: <http://trac.webkit.org/r215592>. This change appears to have broken the CLoop build: https://build.webkit.org/builders/Apple%20Sierra%20LLINT%20CLoop%20%28BuildAndTest%29/builds/1009

Mark Lam

Comment 7 2017-04-20 17:21:40 PDT

(In reply to Ryan Haddad from comment #6) > (In reply to Mark Lam from comment #5) > > Thanks for the reviews. Landed in r215592: <http://trac.webkit.org/r215592>. > > This change appears to have broken the CLoop build: > > https://build.webkit.org/builders/ > Apple%20Sierra%20LLINT%20CLoop%20%28BuildAndTest%29/builds/1009 Build fix for this landed in r215595: <http://trac.webkit.org/r215595>.

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