Bug 175672 - Only use 16 VFP registers if !CPU(ARM_NEON).
Summary: Only use 16 VFP registers if !CPU(ARM_NEON).
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks: 175514
  Show dependency treegraph
 
Reported: 2017-08-17 07:34 PDT by Mark Lam
Modified: 2017-08-17 12:57 PDT (History)
12 users (show)

See Also:


Attachments
proposed patch. (12.04 KB, patch)
2017-08-17 07:35 PDT, Mark Lam
mark.lam: review-
Details | Formatted Diff | Diff
proposed patch. (14.41 KB, patch)
2017-08-17 10:36 PDT, Mark Lam
jfbastien: review+
Details | Formatted Diff | Diff
patch for landing. (15.01 KB, patch)
2017-08-17 11:02 PDT, Mark Lam
no flags Details | Formatted Diff | Diff
patch for landing. (15.01 KB, patch)
2017-08-17 11:06 PDT, Mark Lam
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2017-08-17 07:34:31 PDT
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.
Comment 1 Mark Lam 2017-08-17 07:35:04 PDT
Created attachment 318366 [details]
proposed patch.
Comment 3 Mark Lam 2017-08-17 07:53:08 PDT
Comment on attachment 318366 [details]
proposed patch.

I think this patch is wrong.  I'll read the specs a bit more.
Comment 4 Mark Lam 2017-08-17 10:36:37 PDT
Created attachment 318379 [details]
proposed patch.
Comment 5 JF Bastien 2017-08-17 10:43:40 PDT
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.
Comment 6 Mark Lam 2017-08-17 11:02:48 PDT
Created attachment 318387 [details]
patch for landing.

Thanks for the review.
Comment 7 Radar WebKit Bug Importer 2017-08-17 11:04:52 PDT
<rdar://problem/33944790>
Comment 8 Build Bot 2017-08-17 11:05:44 PDT
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.
Comment 9 Mark Lam 2017-08-17 11:06:55 PDT
Created attachment 318388 [details]
patch for landing.
Comment 10 Build Bot 2017-08-17 11:09:01 PDT
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.
Comment 11 WebKit Commit Bot 2017-08-17 12:57:50 PDT
Comment on attachment 318388 [details]
patch for landing.

Clearing flags on attachment: 318388

Committed r220871: <http://trac.webkit.org/changeset/220871>
Comment 12 WebKit Commit Bot 2017-08-17 12:57:52 PDT
All reviewed patches have been landed.  Closing bug.