[webkit-reviews] review denied: [Bug 101328] MIPS DFG implementation. : [Attachment 181319] DFG for MIPS.

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has denied Balazs Kilvady
<kilvadyb at homejinni.com>'s request for review:
Bug 101328: MIPS DFG implementation.
https://bugs.webkit.org/show_bug.cgi?id=101328

Attachment 181319: DFG for MIPS.
https://bugs.webkit.org/attachment.cgi?id=181319&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=181319&action=review


>>>> Source/JavaScriptCore/dfg/DFGCCallHelpers.h:963
>>>> +
>>> 
>>> Why is all of this behind CPU(MIPS)?  It looks identical to the normal
NUMBER_OF_ARGUMENT_REGISTERS == 4 case. :-/
>> 
>> Thank you for the review. On MIPS we have to give space for the register
stored arguments on stack also so while other (params in 4 regs) arches can
use:
>>	   poke(arg5, 1);
>>	   poke(arg4);
>>	   setupArgumentsWithExecState(arg1, arg2, arg3);
>> on MIPS we have to use:
>>	   poke(arg5, 5);
>>	   poke(arg4, 4);
>>	   setupArgumentsWithExecState(arg1, arg2, arg3);
>> Different stack offset values in poke call.
> 
> Then I would suggest abstracting the poke offset, and using #if's only for
that.  That way, you don't have to duplicate all of that code!
> 
> Keep in mind that this code gets churned *a lot*.  Every time we add a
DFGOperations function with a sufficiently exotic signature, this code gets
touched.  Therefore, we should reduce code duplication as much as possible;
otherwise you guys will have a lot more work to do to keep up with DFG changes.


See above comment.  I still think you should abstract the poke offset rather
than duplicating all of this code.