[webkit-reviews] review denied: [Bug 206361] Reland bytecode checkpoints since bugs have been fixed : [Attachment 387932] Patch

[bugzilla-daemon at webkit.org]

Caio Lima <ticaiolima at gmail.com> has denied  review:
Bug 206361: Reland bytecode checkpoints since bugs have been fixed
https://bugs.webkit.org/show_bug.cgi?id=206361

Attachment 387932: Patch

https://bugs.webkit.org/attachment.cgi?id=387932&action=review




--- Comment #5 from Caio Lima <ticaiolima at gmail.com> ---
Comment on attachment 387932
  --> https://bugs.webkit.org/attachment.cgi?id=387932
Patch

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

r- because I've found a bug.

> Source/JavaScriptCore/jit/SpecializedThunkJIT.h:58
> +	       VirtualRegister src = virtualRegisterForArgument(argument);

Look that this can cause some troubles on existing code.
`CallFrame::argumentOffset` returns the value based on
`CallFrameSlot::firstArgument + argument`, however virtualRegisterForArgument
uses `argument + CallFrame::thisArgumentOffset();`, making the results off by
one. For example, this can cause issues on code generated by `stringCharLoad`,
because the assembly generated by
`jit.loadJSStringArgument(SpecializedThunkJIT::ThisArgument,
SpecializedThunkJIT::regT0);` won't access `thisArgument` correctly (The reason
is because `SpecializedThunkJIT::ThisArgument == -1`). We don't get crashes on
64-bits for `charCodeAt` because it always fallback to slow path (I suspect we
can observe performance regressions for this issue).