150128 – Add MASM_PROBE support for ARM64

Bug 150128 - Add MASM_PROBE support for ARM64

Summary: Add MASM_PROBE support for ARM64

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	JavaScriptCore (show other bugs)
Version:	WebKit Nightly Build
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Mark Lam

URL:
Keywords:

Depends on:
Blocks:

Reported:	2015-10-14 10:53 PDT by Mark Lam
Modified:	2015-10-15 12:56 PDT (History)
CC List:	9 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

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

Description Mark Lam 2015-10-14 10:53:59 PDT

Patch coming.

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

Created attachment 263142 [details]
the patch.

Comment 2 WebKit Commit Bot 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.

Comment 3 Mark Lam 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.

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

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

Comment 5 WebKit Commit Bot 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.

Comment 6 Michael Saboff 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.

Comment 7 Mark Lam 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>.