Patch coming.
<rdar://problem/37467887>
Created attachment 333612 [details] propose patch.
Created attachment 333615 [details] proposed patch.
Comment on attachment 333615 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=333615&action=review > Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:58 > elsif X86 or X86_WIN > subp 8, sp > push a1 > push a0 > - call function > + doSlowPathCall(function) Nit: Why not just do this in the backend? Then you wouldn't need to split this based on architecture. > Source/JavaScriptCore/offlineasm/instructions.rb:259 > + "calljs", > + "callnative", > + "callslowpath", > + "callslowpathvoid" These should be camel case. I guess you have a conflict between the names of these and the macro name for callSlowPath in the offlineAsm. I would either rename that macro to slowPathCall or rename these to something else.
Comment on attachment 333615 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=333615&action=review >> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:58 >> + doSlowPathCall(function) > > Nit: Why not just do this in the backend? > > Then you wouldn't need to split this based on architecture. Because the backend is only supposed to handle CPU instructions and not calling conventions (such as the stack adjustment made for alignment here). >> Source/JavaScriptCore/offlineasm/instructions.rb:259 >> + "callslowpathvoid" > > These should be camel case. I guess you have a conflict between the names of these and the macro name for callSlowPath in the offlineAsm. > > I would either rename that macro to slowPathCall or rename these to something else. Note: the "memfence" instruction above isn't camel cased as "memFence". That said, I'll try camel casing these and see what falls out.
Comment on attachment 333615 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=333615&action=review >>> Source/JavaScriptCore/offlineasm/instructions.rb:259 >>> + "callslowpathvoid" >> >> These should be camel case. I guess you have a conflict between the names of these and the macro name for callSlowPath in the offlineAsm. >> >> I would either rename that macro to slowPathCall or rename these to something else. > > Note: the "memfence" instruction above isn't camel cased as "memFence". That said, I'll try camel casing these and see what falls out. Actually, would you mind if I do all this renaming in a follow up patch? That will lessen the chance that it introduces a typo bug.
Comment on attachment 333615 [details] proposed patch. r=me to do the rename in a follow up. please file a bug though.
Comment on attachment 333615 [details] proposed patch. Thanks for the review. I'll follow up later with a renaming patch for the requested changes.
(In reply to Keith Miller from comment #7) > r=me to do the rename in a follow up. please file a bug though. Follow up patch to be worked on in https://bugs.webkit.org/show_bug.cgi?id=182708.
Comment on attachment 333615 [details] proposed patch. On second thought, I might not want to introduce these macro instructions. I'll rethink this first.
(In reply to Mark Lam from comment #10) > Comment on attachment 333615 [details] > proposed patch. > > On second thought, I might not want to introduce these macro instructions. > I'll rethink this first. I'm going to do the refactoring in parts. I'll remove the macro instructions part from this patch and land everything else first.
Created attachment 333639 [details] proposed patch.
Thanks for the review. Landed in r228402: <http://trac.webkit.org/r228402>.