Bug 175672

Summary:

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

Product:

WebKit

Reporter:

Mark Lam <mark.lam>

Component:

JavaScriptCore

Assignee:

Mark Lam <mark.lam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, buildbot, cdumez, clopez, cmarcelo, commit-queue, dbates, jfbastien, keith_miller, msaboff, saam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

175514

Attachments:

Description	Flags
proposed patch.	mark.lam: review-
proposed patch.	jfbastien: review+
patch for landing.	none
patch for landing.	none

Mark Lam

Reported 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.

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
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Mark Lam

Comment 1 2017-08-17 07:35:04 PDT

Created attachment 318366 [details] proposed patch.

Mark Lam

Comment 2 2017-08-17 07:51:48 PDT

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 3 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.

Mark Lam

Comment 4 2017-08-17 10:36:37 PDT

Created attachment 318379 [details] proposed patch.

JF Bastien

Comment 5 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.

Mark Lam

Comment 6 2017-08-17 11:02:48 PDT

Created attachment 318387 [details] patch for landing. Thanks for the review.

Radar WebKit Bug Importer

Comment 7 2017-08-17 11:04:52 PDT

<rdar://problem/33944790>

Build Bot

Comment 8 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.

Mark Lam

Comment 9 2017-08-17 11:06:55 PDT

Created attachment 318388 [details] patch for landing.

Build Bot

Comment 10 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.

WebKit Commit Bot

Comment 11 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>

WebKit Commit Bot

Comment 12 2017-08-17 12:57:52 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.