Bug 150128 – Add MASM_PROBE support for ARM64

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

Patch coming.

[Mark Lam]

Created attachment 263142 [details]
the patch.

[WebKit Commit Bot]

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 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]

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

[WebKit Commit Bot]

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 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]

The typo fixes were addressed separately in http://webkit.org/b/150181.

Thanks for the review.  Landed in r191130: <http://trac.webkit.org/r191130>.