Bug 109459 - [iOS] Upstream changes to Platform.h
Summary: [iOS] Upstream changes to Platform.h
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-11 10:08 PST by David Kilzer (:ddkilzer)
Modified: 2013-02-11 15:56 PST (History)
7 users (show)

See Also:


Attachments
Patch v1 (3.26 KB, patch)
2013-02-11 10:15 PST, David Kilzer (:ddkilzer)
benjamin: review+
benjamin: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2013-02-11 10:08:27 PST
Upstream iOS changes to Platform.h.
Comment 1 David Kilzer (:ddkilzer) 2013-02-11 10:15:37 PST
Created attachment 187610 [details]
Patch v1
Comment 2 Laszlo Gombos 2013-02-11 10:33:57 PST
Comment on attachment 187610 [details]
Patch v1

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

> Source/WTF/wtf/Platform.h:-621
> -#if PLATFORM(IOS_SIMULATOR)
> -    #define ENABLE_JIT 0
> -    #define ENABLE_YARR_JIT 0

Is the intention to turn on JIT for the simulator as well ? If it is, the patch looks good to me.
Comment 3 David Kilzer (:ddkilzer) 2013-02-11 10:49:06 PST
(In reply to comment #2)
> (From update of attachment 187610 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=187610&action=review
> 
> > Source/WTF/wtf/Platform.h:-621
> > -#if PLATFORM(IOS_SIMULATOR)
> > -    #define ENABLE_JIT 0
> > -    #define ENABLE_YARR_JIT 0
> 
> Is the intention to turn on JIT for the simulator as well ? If it is, the patch looks good to me.

Yes.
Comment 4 Benjamin Poulain 2013-02-11 13:19:15 PST
Comment on attachment 187610 [details]
Patch v1

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

> Source/WTF/wtf/Platform.h:624
>  

Remove the blank line.

> Source/WTF/wtf/Platform.h:627
> +#define ENABLE_JIT 1
> +#define ENABLE_LLINT 1
> +#define ENABLE_YARR_JIT 1

Move those 3 ENABLE with the ones above?

> Source/WTF/wtf/Platform.h:1131
> +#if PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 60000 || PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1080

I'd just add parenthesis here for readability.
Comment 5 David Kilzer (:ddkilzer) 2013-02-11 15:38:37 PST
(In reply to comment #4)
> (From update of attachment 187610 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=187610&action=review
> 
> > Source/WTF/wtf/Platform.h:624
> >  
> 
> Remove the blank line.

Okay.

> > Source/WTF/wtf/Platform.h:627
> > +#define ENABLE_JIT 1
> > +#define ENABLE_LLINT 1
> > +#define ENABLE_YARR_JIT 1
> 
> Move those 3 ENABLE with the ones above?

Actually, all of these are defined below, so they should probably just be removed for "clarity".  (I don't find the definitions below very easy to follow, but better to match other platforms.)

> > Source/WTF/wtf/Platform.h:1131
> > +#if PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 60000 || PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1080
> 
> I'd just add parenthesis here for readability.

Will do.

Thanks!
Comment 6 David Kilzer (:ddkilzer) 2013-02-11 15:56:46 PST
Committed r142537: <http://trac.webkit.org/changeset/142537>