WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
NEW
160295
[ARM] REGRESSION: generateSelfPropertyAccess shouldn't overwrite the constant pool
https://bugs.webkit.org/show_bug.cgi?id=160295
Summary
[ARM] REGRESSION: generateSelfPropertyAccess shouldn't overwrite the constant...
Csaba Osztrogonác
Reported
2016-07-28 08:25:25 PDT
ARMv7 (ARM instruction set) backend is completely broken due to the recent IC related refactoring/development. I got one more regression related to this IC development. generateSelfPropertyAccess() overwrites the constant pool with nops which doesn't belong to IC code, but previous opcodes. It is easy to reproduce this bug on sunspider-1.0/3d-raytrace.js with default settings. (There are only ~100 crashed on
r203785
with other fixes applied: -
https://trac.webkit.org/changeset/203816
-
https://bugs.webkit.org/attachment.cgi?id=284690
from
bug159720
- revert
https://trac.webkit.org/changeset/203272
) Let's see the generated JIT code --------------------------------- Generated Baseline JIT code for intersect#DSuR25:[0xafba61c0->0xafbdd3c0, BaselineFunctionCall, 368], instruction count = 368 Source: function (orig, dir, near, far) { var u = (this.axis + 1) % 3; var v = (this.axis + 2) % 3; var d = dir[this.axis] + this.nu * dir[u] + this.nv * dir[v]; var t = (this.nd - orig[this.axis] - this.nu * orig[u] - this.nv * orig[v]) / d; if (t < near || t > far) return null; var Pu = orig[u] + t * dir[u] - this.eu; var Pv = orig[v] + t * dir[v] - this.ev; var a2 = Pv * this.nu1 + Pu * this.nv1; if (a2 < 0) return null; var a3 = Pu * this.nu2 + Pv * this.nv2; if (a3 < 0) return null; if ((a2 + a3) > 1) return null; return t; } Code at [0xb1506000, 0xb150a794): ... [ 129] get_by_val loc12, arg1, loc13 ArrayWithDouble, Original; predicting Nonintasdouble 0xb1506b60: [0xe5860000] ldr r6, [pc, #2152] <============ read from constant pool adress: 0xb15073d0 0xb1506b64: [0xe59f6864] str r0, [r6] <============ CRASH, because the address was overwritten 0xb1506b68: [0xe5861000] ldr r6, [pc, #2148] 0xb1506b6c: [0xe50b0068] str r1, [r6] 0xb1506b70: [0xe50b1064] str r0, [r11, #-104] 0xb1506b74: [0xe51b0060] str r1, [r11, #-100] [ 135] sub loc11, loc11, loc12 results: Result:<Int32> LHS ObservedType:<Number> RHS ObservedType:<Number> LHS ResultType:<0x3e> RHS ResultType:<0x3e> ... [ 227] get_by_id loc12, this, eu(@id4) llint(struct = 0xafba3a40 (offset = 5)) predicting Nonboolint32 0xb1507388: [0xe59b1024] ldr r0, [r11, #32] 0xb150738c: [0xea00090c] ldr r1, [r11, #36] 0xb1507390: [0xe1a00000] b #9264 0xb1507394: [0xe1a00000] mov r0, r0 0xb1507398: [0xe1a00000] mov r0, r0 0xb150739c: [0xe1a00000] mov r0, r0 0xb15073a0: [0xe1a00000] mov r0, r0 0xb15073a4: [0xe1a00000] mov r0, r0 0xb15073a8: [0xe1a00000] mov r0, r0 0xb15073ac: [0xe1a00000] mov r0, r0 0xb15073b0: [0xe1a00000] mov r0, r0 0xb15073b4: [0xe1a00000] mov r0, r0 0xb15073b8: [0xe1a00000] mov r0, r0 0xb15073bc: [0xe1a00000] mov r0, r0 0xb15073c0: [0xea000077] mov r0, r0 0xb15073c4: [0x00002c8c] b #476 <================== constant pool starts with barrier 0xb15073c8: [0x00000b60] andeq r2, r0, r12, lsl #25 0xb15073cc: [0xaffebb58] andeq r0, r0, r0, ror #22 0xb15073d0: [0xaffebb5c] svcge #16694104 <================== read from here before the CRASH 0xb15073d4: [0x00000bb0] svcge #16694108 0xb15073d8: [0x00000be0] unknown instruction ... Let's see what's going wrong ----------------------------- generating 492 nops from this backtrace which overwrite the constant pool 1 0xb58087a4 JSC::LinkBuffer::allocate(JSC::MacroAssembler&, void*, JSC::JITCompilationEffort) 2 0xb58085f8 JSC::LinkBuffer::linkCode(JSC::MacroAssembler&, void*, JSC::JITCompilationEffort) 3 0xb58bef1c JSC::LinkBuffer::LinkBuffer(JSC::MacroAssembler&, void*, unsigned int, JSC::JITCompilationEffort, bool) 4 0xb58bd654 5 0xb58bca80 JSC::InlineAccess::generateSelfPropertyAccess(JSC::VM&, JSC::StructureStubInfo&, JSC::Structure*, int) 6 0xb5f81550 7 0xb5f82024 JSC::repatchGetByID(JSC::ExecState*, JSC::JSValue, JSC::Identifier const&, JSC::PropertySlot const&, JSC::StructureStubInfo&, JSC::GetByIDKind) 8 0xb5f48338 9 0xb5f54308 10 0xb5f53670 11 0xb5f48528 Generated JIT code for InlineAccessType: 'property access': Code at [0xb1507390, 0xb15075a8): 0xb1507390: [0xe3036a40] ldr r12, [r0] 0xb1507394: [0xe34a6fba] movw r6, #14912 0xb1507398: [0xe15c0006] movt r6, #44986 0xb150739c: [0x159fc010] cmp r12, r6 0xb15073a0: [0x112fff1c] ldrne r12, [pc, #16] 0xb15073a4: [0xe590103c] bxne r12 0xb15073a8: [0xe5900038] ldr r1, [r0, #60] 0xb15073ac: [0xea000001] ldr r0, [r0, #56] 0xb15073b0: [0xe12fff7f] b #4 0xb15073b4: [0xb15097c8] bkpt #65535 0xb15073b8: [0xe1a00000] unknown instruction 0xb15073bc: [0xe1a00000] mov r0, r0 0xb15073c0: [0xe1a00000] mov r0, r0 0xb15073c4: [0xe1a00000] mov r0, r0 < ============= constant pool barrier should be here, but was overwritten 0xb15073c8: [0xe1a00000] mov r0, r0 ... 0xb15075a0: [0xe1a00000] mov r0, r0 0xb15075a4: [0xe59f68e0] mov r0, r0 Have you got any idea how can we fix this serious regression? It seems the new IC mechanism doesn't respect constant pools at all. :( My first idea is that we should force flush constant pool before generating any code for get_by_id. Do you think if it would help? And could you help where should I put this flush instruction?
Attachments
Noisy Patch
(11.67 KB, patch)
2017-08-01 17:06 PDT
,
Caio Lima
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1
2016-07-29 00:57:42 PDT
Could you give me some hints how can we fix this regression?
Csaba Osztrogonác
Comment 2
2016-07-29 11:08:34 PDT
ping?
Saam Barati
Comment 3
2016-08-07 14:21:59 PDT
So this happens when we're regenerating the IC? We don't want to take into account the constant pool into the size of the IC. Is there a predictable way to predict the size of a constant pool for code with X number of branches when X is well known?
Caio Lima
Comment 4
2017-08-01 17:02:48 PDT
(In reply to Saam Barati from
comment #3
)
> So this happens when we're regenerating the IC?
No. The problem is happening when the getById fast path is being generated in "JITByIdGenerator::generateFastCommon" and the constant poll is flushed in the middle of IC code. As the logic in "JSC::LinkBuffer::allocate" is to fill al remaining IC code with nops, constant pool is then overwritten in such case. However, it also could be overwritten by IC repatch version as well.
> We don't want to take into account the constant pool into the size of the IC.
I've found a solution that I'm not happy with, but at least enables me run code with IC enabled.
Caio Lima
Comment 5
2017-08-01 17:06:37 PDT
Created
attachment 316916
[details]
Noisy Patch This Patch doesn't apply to master. Also, it's is just the hacking I've made to identify the problem. Maybe it can be useful to share. However, I'm evaluating if enable JIT into ARMv6 will provide performance gain as we expect and if we succeed on that I can revisit this bug afterwards.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug