Bug 150128

Summary:

Add MASM_PROBE support for ARM64

Product:

WebKit

Reporter:

Mark Lam <mark.lam>

Component:

JavaScriptCore

Assignee:

Mark Lam <mark.lam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, commit-queue, fpizlo, ggaren, keith_miller, msaboff, ossy, saam, ysuzuki

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
the patch.	mark.lam: commit-queue-
patch 2: removed stray debugging code.	msaboff: review+, msaboff: commit-queue-

Mark Lam

Reported 2015-10-14 10:53:59 PDT

Patch coming.

Attachments
the patch. (53.58 KB, patch) 2015-10-15 00:29 PDT, Mark Lam	mark.lam: commit-queue-	Details Formatted Diff Diff
patch 2: removed stray debugging code. (53.55 KB, patch) 2015-10-15 10:10 PDT, Mark Lam	msaboff: review+ msaboff: commit-queue-	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Mark Lam

Comment 1 2015-10-15 00:29:11 PDT

Created attachment 263142 [details] the patch.

WebKit Commit Bot

Comment 2 2015-10-15 00:30:52 PDT

Attachment 263142 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:401: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:410: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:412: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:420: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:423: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:436: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:450: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:459: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:468: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:487: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:489: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:499: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:501: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JITStubsARM64.h:450: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 15 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.

Mark Lam

Comment 3 2015-10-15 09:56:59 PDT

Comment on attachment 263142 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=263142&action=review > Source/JavaScriptCore/assembler/MacroAssemblerARM64.cpp:54 > + dataLogF(" *** x18 %p\n", context->cpu.x18); Eek. This is debugging code that I need to remove before landing.

Mark Lam

Comment 4 2015-10-15 10:10:30 PDT

Created attachment 263169 [details] patch 2: removed stray debugging code.

WebKit Commit Bot

Comment 5 2015-10-15 10:13:52 PDT

Attachment 263169 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:401: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:410: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:412: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:420: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:423: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:425: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:436: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:450: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:459: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:468: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:487: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:489: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:499: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/assembler/ARM64Assembler.h:501: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/jit/JITStubsARM64.h:450: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 15 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.

Michael Saboff

Comment 6 2015-10-15 11:52:57 PDT

Comment on attachment 263169 [details] patch 2: removed stray debugging code. View in context: https://bugs.webkit.org/attachment.cgi?id=263169&action=review I'm not too fond of the complications to the register definitions, however the other architectures that have probe support are structured similarly. r=me > Source/JavaScriptCore/jit/JITStubsARM.h:114 > -COMPILE_ASSERT(PROBE_OFFSETOF(cpu.r7) == PROBE_CPU_R7_OFFSET, ProbeContext_cpu_r70_offset_matches_ctiMasmProbeTrampoline); > +COMPILE_ASSERT(PROBE_OFFSETOF(cpu.r7) == PROBE_CPU_R7_OFFSET, ProbeContext_cpu_r7_offset_matches_ctiMasmProbeTrampoline); These changes should be in a separate patch, which I will RS. > Source/JavaScriptCore/jit/JITStubsARMv7.h:129 > -COMPILE_ASSERT(PROBE_OFFSETOF(cpu.r7) == PROBE_CPU_R7_OFFSET, ProbeContext_cpu_r70_offset_matches_ctiMasmProbeTrampoline); > +COMPILE_ASSERT(PROBE_OFFSETOF(cpu.r7) == PROBE_CPU_R7_OFFSET, ProbeContext_cpu_r7_offset_matches_ctiMasmProbeTrampoline); Ditto. > Source/JavaScriptCore/jit/JITStubsARMv7.h:264 > - // pc from there. > + // pc from there. Ditto. > Source/JavaScriptCore/jit/JITStubsARMv7.h:267 > - // ProbeContext.cpu.sp - 4. But if the user probe function had modified > + // ProbeContext.cpu.sp - 4. But if the user probe function had modified Ditto.

Mark Lam

Comment 7 2015-10-15 12:56:50 PDT

The typo fixes were addressed separately in http://webkit.org/b/150181. Thanks for the review. Landed in r191130: <http://trac.webkit.org/r191130>.

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