[Webkit-unassigned] [Bug 104377] [sh4] Implement add64 for SH4 assembler after r136601

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=104377





--- Comment #2 from Zoltan Herczeg <zherczeg at webkit.org>  2012-12-11 03:06:49 PST ---
(From update of attachment 178484)
Looks good, few comments:

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

> Source/JavaScriptCore/assembler/MacroAssemblerSH4.h:410
> +        // add 32-bit LSB first

WebKit comment style is different. Please use full sentences.

> Source/JavaScriptCore/assembler/MacroAssemblerSH4.h:411
> +        m_assembler.loadConstant(reinterpret_cast<uint32_t>(address.m_ptr), scr1); // src1 = int64 address

In WebKit you don't need to explain trivial facts :) Too much comments makes the code less readable.
See http://www.webkit.org/coding/coding-style.html comments section.

> Source/JavaScriptCore/assembler/MacroAssemblerSH4.h:416
> +        m_assembler.loadConstant(reinterpret_cast<uint32_t>(address.m_ptr), scr1); // src1 = int64 address

I am not sure how SH4 works, but if this requires a memory load, a third scratch register could be used to store the address of int64. I don't know you have a third scratch, so this might be unavoidable.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.