Bug 159408

Summary: Fix the ARMv7 platform with ARM instruction set after r202214
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: benjamin, cdumez, cgarcia, clopez, cmarcelo, commit-queue, fpizlo, keith_miller, mark.lam, msaboff, ossy, sbarati, tpopela
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 159709, 159720, 159758, 160295, 123717, 159707, 159708, 159711, 159713, 159759    
Bug Blocks: 108645, 158719, 159083    
Attachments:
Description Flags
Patch
none
Patch none

Description Csaba Osztrogonác 2016-07-04 08:02:17 PDT
http://trac.webkit.org/changeset/202214 broke the ARMv7 build with ARM instruction set 
(ARM traditional) because of missing patchableBranch32 and fillNops implementations.

bug159083 fixed the build in a wrong way - switching to Thumb2 explicitly.
I'm going to revert this change and add missing functions.
Comment 1 Csaba Osztrogonác 2016-07-04 08:05:59 PDT
Created attachment 282721 [details]
Patch

WIP patch. It builds, but tests crashes now, still debugging.
Comment 2 Csaba Osztrogonác 2016-07-04 08:09:03 PDT
Comment on attachment 282721 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=282721&action=review

> Source/JavaScriptCore/bytecode/InlineAccess.h:56
> -        return 50;
> +        return 64;

WIP: Return dummy but valid values, until we can calculate the proper value somehow.
I hope more nops won't cause problems.

> Source/JavaScriptCore/bytecode/InlineAccess.h:75
> -        return 50;
> +        return 64;

ditto

> Source/JavaScriptCore/bytecode/InlineAccess.h:94
> -        return 50;
> +        return 64;

ditto
Comment 3 Csaba Osztrogonác 2016-07-04 08:10:20 PDT
infos about the crash I notice:

sunspider-1.0/3d-cube.js.dfg-maximal-flush-validate-no-cjit:     [1470] get_by_id         loc9, loc8, NumPx(@id22) llint(struct = 0xb29a7ae0 (offset = 103))    predicting Nonboolint32
sunspider-1.0/3d-cube.js.dfg-maximal-flush-validate-no-cjit:         disassembly not available for range 0xb2f13da4...0xb2f13e0c

(gdb) disas 0xb2f13da4,0xb2f13e0c
Dump of assembler code from 0xb2f13da4 to 0xb2f13e0c:
   0xb2f13da4:  ldr     r0, [r11, #-72] ; 0x48
   0xb2f13da8:  ldr     r1, [r11, #-68] ; 0x44
   0xb2f13dac:  cmn     r1, #5
   0xb2f13db0:  bne     0xb2f18c48
   0xb2f13db4:  ldr     r12, [r0]
   0xb2f13db8:  movw    r6, #31456      ; 0x7ae0
   0xb2f13dbc:  movt    r6, #45722      ; 0xb29a
   0xb2f13dc0:  cmp     r12, r6
   0xb2f13dc4:  ldrne   r12, [pc, #16]  ; 0xb2f13ddc
   0xb2f13dc8:  bxne    r12
   0xb2f13dcc:  ldr     r0, [r0, #8]
   0xb2f13dd0:  ldr     r1, [r0, #-36]  ; 0x24
   0xb2f13dd4:  ldr     r0, [r0, #-40]  ; 0x28
=> 0xb2f13dd8:  bkpt    0xffff
   0xb2f13ddc:  rscslt  r8, r1, #72, 24 ; 0x4800
   0xb2f13de0:  nop                     ; (mov r0, r0)
   0xb2f13de4:  nop                     ; (mov r0, r0)
   0xb2f13de8:  nop                     ; (mov r0, r0)
   0xb2f13dec:  nop                     ; (mov r0, r0)
   0xb2f13df0:  nop                     ; (mov r0, r0)
   0xb2f13df4:  ldr     r6, [pc, #1848] ; 0xb2f14534
   0xb2f13df8:  str     r0, [r6]
   0xb2f13dfc:  ldr     r6, [pc, #1844] ; 0xb2f14538
   0xb2f13e00:  str     r1, [r6]
   0xb2f13e04:  str     r0, [r11, #-80] ; 0x50
   0xb2f13e08:  str     r1, [r11, #-76] ; 0x4c
End of assembler dump.
(gdb) info registers
r0             0x168    360
r1             0xffffffff       4294967295
r2             0x0      0
r3             0xbed44828       3201583144
r4             0xffffffff       4294967295
r5             0x0      0
r6             0xb29a7ae0       2996468448
r7             0x0      0
r8             0xb29c71e0       2996597216
r9             0xfffffffb       4294967291
r10            0x1      1
r11            0xbed44968       3201583464
r12            0xb29a7ae0       2996468448
sp             0xbed448a0       0xbed448a0
lr             0xb2f13d1c       -1292813028
pc             0xb2f13dd8       0xb2f13dd8
cpsr           0x600f0010       1611595792
Comment 4 Csaba Osztrogonác 2016-07-05 04:09:33 PDT
Created attachment 282767 [details]
Patch

Still WIP patch, updated magic numbers, still have crashes.
Comment 5 Saam Barati 2016-07-05 08:40:58 PDT
Comment on attachment 282767 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=282767&action=review

> Source/JavaScriptCore/assembler/MacroAssemblerARM.h:1550
> +            m_assembler.cmp(ARMRegisters::S1, m_assembler.getImm(right.m_value, ARMRegisters::S0));

Are S0 and S1 always scratch? As in it will never be a register that DFG allocates?
Comment 6 Csaba Osztrogonác 2016-07-06 02:20:22 PDT
(In reply to comment #5)
> Comment on attachment 282767 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=282767&action=review
> 
> > Source/JavaScriptCore/assembler/MacroAssemblerARM.h:1550
> > +            m_assembler.cmp(ARMRegisters::S1, m_assembler.getImm(right.m_value, ARMRegisters::S0));
> 
> Are S0 and S1 always scratch? As in it will never be a register that DFG
> allocates?

As far as I know we can use scratch registers in MacroAssembler
without any restriction. But I might be wrong.

note: I'll update magic number based on
https://bugs.webkit.org/show_bug.cgi?id=158719#c29
Comment 7 Csaba Osztrogonác 2016-07-13 03:22:36 PDT
I found more bugs here, I'm going to fix them one by one in separated bugs.