Bug 175672 – Only use 16 VFP registers if !CPU(ARM_NEON).

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

We should not assume the existence of 32 VFP registers in the absence of CPU(ARM_NEON).  This contributes to a build failure on GTK for ARM_THUMB2.  See https://bugs.webkit.org/show_bug.cgi?id=175514.

[Mark Lam]

Created attachment 318366 [details]
proposed patch.

[Mark Lam]

Some more data:

The NEON view of floating point registers:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0473c/CJACABEJ.html

The VFPv3 and VFPv2 view of floating point registers:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0473c/CIHDIBDG.html
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0473c/Bcfhdhgd.html

[Mark Lam]

Comment on attachment 318366 [details]
proposed patch.

I think this patch is wrong.  I'll read the specs a bit more.

[Mark Lam]

Created attachment 318379 [details]
proposed patch.

[JF Bastien]

Comment on attachment 318379 [details]
proposed patch.

r=me

Instead of saying NEON everywhere and having the "assume..." comment, I'd rather have #if NEON in one place, and *there* the assume comment which defines VFPv3D32 instead, and then everywhere else you do #if VFPv3D32.

Note that there's also VFPv3-D16 ! Hende the D32 distinction I make above. I think your assumption about NEON is the right thing, I'm just being pedantic because technically VFPv3-D16 doesn't have the extra registers either.

[Mark Lam]

Created attachment 318387 [details]
patch for landing.

Thanks for the review.

[Radar WebKit Bug Importer]

<rdar://problem/33944790>

[Build Bot]

Attachment 318387 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Platform.h:342:  CPP comments are not allowed in Platform.h, please use C comments /* ... */  [build/cpp_comment] [5]
ERROR: Source/WTF/wtf/Platform.h:343:  CPP comments are not allowed in Platform.h, please use C comments /* ... */  [build/cpp_comment] [5]
ERROR: Source/WTF/wtf/Platform.h:344:  CPP comments are not allowed in Platform.h, please use C comments /* ... */  [build/cpp_comment] [5]
ERROR: Source/WTF/wtf/Platform.h:345:  CPP comments are not allowed in Platform.h, please use C comments /* ... */  [build/cpp_comment] [5]
ERROR: Source/WTF/wtf/Platform.h:346:  CPP comments are not allowed in Platform.h, please use C comments /* ... */  [build/cpp_comment] [5]
ERROR: Source/WTF/wtf/Platform.h:347:  CPP comments are not allowed in Platform.h, please use C comments /* ... */  [build/cpp_comment] [5]
Total errors found: 6 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

Created attachment 318388 [details]
patch for landing.

[Build Bot]

Attachment 318388 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Platform.h:345:  CPP comments are not allowed in Platform.h, please use C comments /* ... */  [build/cpp_comment] [5]
ERROR: Source/WTF/wtf/Platform.h:346:  CPP comments are not allowed in Platform.h, please use C comments /* ... */  [build/cpp_comment] [5]
ERROR: Source/WTF/wtf/Platform.h:347:  CPP comments are not allowed in Platform.h, please use C comments /* ... */  [build/cpp_comment] [5]
Total errors found: 3 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 318388 [details]
patch for landing.

Clearing flags on attachment: 318388

Committed r220871: <http://trac.webkit.org/changeset/220871>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.