Bug 46096 - Enable ARM VFP on hardware that defines VFP and isn't LINUX
Summary: Enable ARM VFP on hardware that defines VFP and isn't LINUX
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-20 08:39 PDT by David Tapuska
Modified: 2010-10-25 12:30 PDT (History)
7 users (show)

See Also:


Attachments
Add define to test if VFP is enabled by RVCT compiler (1.48 KB, patch)
2010-09-20 10:16 PDT, David Tapuska
ddkilzer: review-
ddkilzer: commit-queue-
Details | Formatted Diff | Diff
Add GCC defines to VFP present setting (1.54 KB, patch)
2010-10-25 06:51 PDT, David Tapuska
no flags Details | Formatted Diff | Diff
Add GCC defines to VFP present setting preserving line 59 (1.54 KB, patch)
2010-10-25 08:57 PDT, David Tapuska
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Tapuska 2010-09-20 08:39:59 PDT
The isVFPPresent implementation of MacroAssemblerARM.cpp could take into account the current compiler setting for setting the default of whether a VFP is present or not.
Comment 1 David Tapuska 2010-09-20 10:16:24 PDT
Created attachment 68106 [details]
Add define to test if VFP is enabled by RVCT compiler
Comment 2 Zoltan Herczeg 2010-09-20 11:57:15 PDT
Would be good to add the same condition for GCC as well.
Comment 3 David Tapuska 2010-09-20 11:59:58 PDT
(In reply to comment #2)
> Would be good to add the same condition for GCC as well.

I'd be happy to add it but do you know what it defines when hardware FPU is set? I couldn't find the appropriate documentation on it.
Comment 4 David Kilzer (:ddkilzer) 2010-10-24 08:44:15 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > Would be good to add the same condition for GCC as well.
> 
> I'd be happy to add it but do you know what it defines when hardware FPU is set? I couldn't find the appropriate documentation on it.

It appears that Apple's gcc-4.2 compilers use __VFP_FP__:

$ gcc-4.2 -x c -arch armv6 -isysroot /Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS4.2.sdk -dM -E /dev/null | grep -i vfp
#define __VFP_FP__ 1

$ gcc-4.2 -x c -arch armv7 -isysroot /Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS4.2.sdk -dM -E /dev/null | grep -i vfp
#define __VFP_FP__ 1
Comment 5 David Kilzer (:ddkilzer) 2010-10-24 08:45:06 PDT
Comment on attachment 68106 [details]
Add define to test if VFP is enabled by RVCT compiler

r- to include support for gcc as well.  Otherwise this looks good!
Comment 6 David Kilzer (:ddkilzer) 2010-10-24 08:53:02 PDT
Comment on attachment 68106 [details]
Add define to test if VFP is enabled by RVCT compiler

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

> JavaScriptCore/assembler/MacroAssemblerARM.cpp:59
> +#if defined(__TARGET_FPU_VFP)

Also, I think you should add compiler checks:

    #if COMPILER(RVCT) && defined(__TARGET_FPU_VFP) || COMPILER(GCC) && defined(__VFP_FP__)
Comment 7 David Tapuska 2010-10-25 06:51:07 PDT
Created attachment 71742 [details]
Add GCC defines to VFP present setting
Comment 8 WebKit Review Bot 2010-10-25 06:52:07 PDT
Attachment 71742 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
JavaScriptCore/assembler/MacroAssemblerARM.cpp:59:  More than one command on the same line in if  [whitespace/parens] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 David Kilzer (:ddkilzer) 2010-10-25 08:36:50 PDT
Comment on attachment 71742 [details]
Add GCC defines to VFP present setting

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

Thanks!  r=me

> JavaScriptCore/assembler/MacroAssemblerARM.cpp:-59
> -

Nit: I would leave this blank line in place.
Comment 10 David Tapuska 2010-10-25 08:57:45 PDT
Created attachment 71755 [details]
Add GCC defines to VFP present setting preserving line 59
Comment 11 WebKit Review Bot 2010-10-25 08:59:16 PDT
Attachment 71755 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
JavaScriptCore/assembler/MacroAssemblerARM.cpp:60:  More than one command on the same line in if  [whitespace/parens] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 David Kilzer (:ddkilzer) 2010-10-25 09:05:54 PDT
Comment on attachment 71755 [details]
Add GCC defines to VFP present setting preserving line 59

r=me
Comment 13 Eric Seidel (no email) 2010-10-25 11:31:39 PDT
Comment on attachment 71742 [details]
Add GCC defines to VFP present setting

Cleared David Kilzer's review+ from obsolete attachment 71742 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 14 WebKit Commit Bot 2010-10-25 12:27:57 PDT
The commit-queue encountered the following flaky tests while processing attachment 71755 [details]:

http/tests/appcache/fail-on-update.html

Please file bugs against the tests.  The author(s) of the test(s) are ap@webkit.org, ap@webkit.org, and ap@webkit.org.  The commit-queue is continuing to process your patch.
Comment 15 WebKit Commit Bot 2010-10-25 12:30:24 PDT
Comment on attachment 71755 [details]
Add GCC defines to VFP present setting preserving line 59

Clearing flags on attachment: 71755

Committed r70474: <http://trac.webkit.org/changeset/70474>
Comment 16 WebKit Commit Bot 2010-10-25 12:30:31 PDT
All reviewed patches have been landed.  Closing bug.