We end up producing code like this: 45:<!1:loc6> Call(@42, @36, JS|MustGen|VarArgs|PureInt, Other, R:World, W:Heap, ExitsForExceptions, ClobbersExit, bc#33) predicting Other 0x24ab9c3ffd2c: mov $0x1, 0x10(%rsp) 0x24ab9c3ffd34: mov $0x1, 0x10(%rsp) Which is stupid.
Created attachment 265560 [details] patch
Comment on attachment 265560 [details] patch r=me
This patch fixed the issue of setting the argument count twice. We also needs to set the argument count when making a tail call. If we aren't, then file a bug.
(In reply to comment #3) > This patch fixed the issue of setting the argument count twice. > > We also needs to set the argument count when making a tail call. If we > aren't, then file a bug. We should be setting it on the caller's call frame, right?
It looks like it's being set: ``` 49:<!0:-> TailCall(@28, @23, @31, @34, @37, @40, @43, MustGen|VarArgs, R:World, W:SideState, Exits, bc#36) 0x2268c3e00287: mov $0x10d4685b0, %rcx 0x2268c3e00291: mov $0x0, 0x24(%rbp) 0x2268c3e00298: mov $0x0, %r11 0x2268c3e002a2: cmp %r11, %rcx 0x2268c3e002a5: jnz 0x2268c3e00362 0x2268c3e002ab: mov 0x20(%rbp), %edi 0x2268c3e002ae: cmp $0x4, %edi 0x2268c3e002b1: jbe 0x2268c3e002c8 0x2268c3e002b7: add $0x6, %edi 0x2268c3e002ba: and $0xfffffffe, %edi 0x2268c3e002bd: imul $0x8, %edi, %edi 0x2268c3e002c3: jmp 0x2268c3e002cd 0x2268c3e002c8: mov $0x50, %edi 0x2268c3e002cd: add %rbp, %rdi 0x2268c3e002d0: sub $0x58, %rdi 0x2268c3e002d4: mov 0x0(%rbp), %rbp 0x2268c3e002d8: mov $0x10d43f700, %r11 0x2268c3e002e2: mov %r11, 0x20(%rdi) 0x2268c3e002e6: mov %rax, 0x28(%rdi) 0x2268c3e002ea: mov %rsi, 0x30(%rdi) 0x2268c3e002ee: mov %rdx, 0x38(%rdi) 0x2268c3e002f2: mov $0xffff00000000001e, %r11 0x2268c3e002fc: mov %r11, 0x40(%rdi) 0x2268c3e00300: mov $0xffff000000000028, %r11 0x2268c3e0030a: mov %r11, 0x48(%rdi) 0x2268c3e0030e: movsd 0xb8(%rsp), %xmm0 0x2268c3e00317: mov %rcx, 0x10(%rdi) 0x2268c3e0031b: mov 0xa8(%rsp), %r15 0x2268c3e00323: movsd %xmm0, (%rdi) 0x2268c3e00327: mov 0x88(%rsp), %rbx 0x2268c3e0032f: mov 0x90(%rsp), %r12 0x2268c3e00337: mov 0x98(%rsp), %r13 0x2268c3e0033f: mov 0xa0(%rsp), %r14 0x2268c3e00347: mov $0x0, 0x1c(%rdi) 0x2268c3e0034e: mov $0x6, 0x18(%rdi) 0x2268c3e00355: mov %rdi, %rsp 0x2268c3e00358: jmp 0x2268c3e0035d 0x2268c3e0035d: jmp 0x2268c3e003d9 0x2268c3e00362: mov $0x10d43f700, %r11 0x2268c3e0036c: mov %r11, 0x18(%rsp) 0x2268c3e00371: mov %rax, 0x20(%rsp) 0x2268c3e00376: mov %rsi, 0x28(%rsp) 0x2268c3e0037b: mov %rdx, 0x30(%rsp) 0x2268c3e00380: mov $0xffff00000000001e, %r11 0x2268c3e0038a: mov %r11, 0x38(%rsp) 0x2268c3e0038f: mov $0xffff000000000028, %r11 0x2268c3e00399: mov %r11, 0x40(%rsp) 0x2268c3e0039e: mov %rcx, 0x8(%rsp) 0x2268c3e003a3: mov -0x28(%rbp), %rbx 0x2268c3e003a7: mov -0x20(%rbp), %r12 0x2268c3e003ab: mov -0x18(%rbp), %r13 0x2268c3e003af: mov -0x10(%rbp), %r14 0x2268c3e003b3: mov -0x8(%rbp), %r15 0x2268c3e003b7: mov $0x0, 0x14(%rsp) 0x2268c3e003bf: mov $0x6, 0x10(%rsp) 0x2268c3e003c7: mov %rcx, %rax 0x2268c3e003ca: mov $0x10d7c7f80, %rdx 0x2268c3e003d4: call 0x226883e01c60 0x2268c3e003d9: mov $0xed, %r11d 0x2268c3e003df: int3 ``` which is from this program: ``` "use strict"; function bar() {} noInline(bar); function foo(a, b, c) { return bar(a, b, c, 30, 40); } noInline(foo); for (let i = 0; i < 1000; i++) foo(true, true, 10); ``` Maybe the call frame shuffler takes care of setting the argument count?
To clarify what I mean in the bug title: When I say tail calls don't need to set the argument count, I mean they don't need to set it for a "callee" they're setting up, tail calls need to set it on the frame they're reusing.
Comment on attachment 265560 [details] patch Clearing flags on attachment: 265560 Committed r192465: <http://trac.webkit.org/changeset/192465>
All reviewed patches have been landed. Closing bug.