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?
Could you give me some hints how can we fix this regression?
ping?
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?
(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.
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.