[Webkit-unassigned] [Bug 90198] Port DFG JIT to traditional ARM

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from Filip Pizlo <fpizlo at apple.com>  2012-07-04 01:24:52 PST ---
(In reply to comment #10)
> (From update of attachment 150590 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150590&action=review
> 
> >> Source/JavaScriptCore/assembler/ARMAssembler.cpp:295
> >> +    if (offset == 0) {
> > 
> > Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
> 
> You should refine this and other style errors below. ;)
> 
> > Source/JavaScriptCore/assembler/ARMAssembler.h:-158
> > -#if WTF_ARM_ARCH_AT_LEAST(5) || defined(__ARM_ARCH_4T__)
> 
> Please mention in the ChangeLog that the support of ARMv4 and below is removed.
> 
> Additionally you should check WTF_ARM_ARCH_AT_LEAST(4) and similar constructs and modify them in Platform.h and other files.
> 
> > Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h:269
> > +#if CPU(ARM_THUMB2)
> >          m_assembler.vmov(payloadGPR, tagGPR, fpr);
> > +#else
> > +        m_assembler.vmov_arm64_r(payloadGPR, tagGPR, fpr);
> > +#endif
> 
> I suggest to define a common name or a wrapper function reducing #ifdef burden for the same logic.
> 
> Apart from these, I really like your patch. ;)

+1

I like the idea of wrapping the "m_assembler.vmov_blah" in something nicer.  Perhaps you can add a method in DFGAssemblyHelpers.h like:

void transferReturnValueToReturnValueFPR()
{
#if thumb2
     the thumb2 thingy
#else if arm
     the arm thingy
#else
     do thing
#endif
}

I think that would be useful in other places in the future, particularly since there are code paths in the DFG that make calls in a manner that bypasses the helper methods for a variety of reasons.  Currently none of those deal with floating point returns, but that might change.

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