Bug 80626

Summary: [chromium] Only build NEON files if target_arch=="arm"
Product: WebKit Reporter: Nico Weber <thakis>
Component: New BugsAssignee: Nico Weber <thakis>
Status: RESOLVED FIXED    
Severity: Normal CC: raymes, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Nico Weber 2012-03-08 12:15:42 PST
[chromium] Only build NEON files fi target_arch=="arm"
Comment 1 Nico Weber 2012-03-08 12:18:01 PST
Created attachment 130874 [details]
Patch
Comment 2 Tony Chang 2012-03-08 12:28:08 PST
Comment on attachment 130874 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Currently, webcore_arm_neon will compile a bunch of files
> +        whose contents are completely ifdef'd away on non-arm, and
> +        then bundle all the generated empty .o files into a useless
> +        libwebcore_arm_neon.a. Don't do this.

This change seems fine since we're not using the lib at all.

> Source/WebCore/ChangeLog:12
> +        Also change the source section from "include everything, then
> +        throw away everything except 6 files" to "list 6 files".

I don't like this because this means we have a file list in two places (confusing for non-chromium people adding new files).  The current design is to have all the files listed WebCore.gypi.  Another way to do this would be to make a new variable in WebCore.gypi that lists the NEON files.
Comment 3 Nico Weber 2012-03-08 12:31:36 PST
Created attachment 130879 [details]
Patch
Comment 4 Nico Weber 2012-03-08 12:32:35 PST
> > Source/WebCore/ChangeLog:12
> > +        Also change the source section from "include everything, then
> > +        throw away everything except 6 files" to "list 6 files".
> 
> I don't like this because this means we have a file list in two places (confusing for non-chromium people adding new files).  The current design is to have all the files listed WebCore.gypi.  Another way to do this would be to make a new variable in WebCore.gypi that lists the NEON files.

Reverted that part. Maybe I'll do your suggestin in a follow-up (for platform/ and rendering/, too)
Comment 5 WebKit Review Bot 2012-03-08 19:21:19 PST
Comment on attachment 130879 [details]
Patch

Clearing flags on attachment: 130879

Committed r110251: <http://trac.webkit.org/changeset/110251>
Comment 6 WebKit Review Bot 2012-03-08 19:21:23 PST
All reviewed patches have been landed.  Closing bug.