Bug 67299 – Add Android's platform specification and atomic functions

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Peter Beverloo]

This will re-add the Platform.h changes removed in r94191, and adds Android's atomic functions to WTF.

[Peter Beverloo]

Created attachment 105790 [details]
Patch

[Steve Block]

Comment on attachment 105790 [details]
Patch

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

LGTM, but I'd like to get somebody else to take a look to double-check the change to Platform.h, perhaps Maciej?

Also, shouldn't this bug block Bug 66689?

> Source/JavaScriptCore/wtf/Atomics.h:109
> +inline int atomicDecrement(int volatile* addend) { return __atomic_dec(addend); }

So __atomic_inc/dec() are functionally identical to android_atomic_inc/dec() removed in r94191, but are in the NDK, right?

> Source/JavaScriptCore/wtf/Platform.h:306
> +/* OS(ANDRO67299ID) - Android */

??!!

[Peter Beverloo]

Created attachment 105805 [details]
Patch

[Peter Beverloo]

(In reply to comment #2)
> (From update of attachment 105790 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=105790&action=review
> 
> LGTM, but I'd like to get somebody else to take a look to double-check the change to Platform.h, perhaps Maciej?

Ok, thanks.

> 
> Also, shouldn't this bug block Bug 66689?

Yes, you're right. I've added the bug, but won't remove the main one as that'll unnecessarily mail all cc'ed people again.

> 
> > Source/JavaScriptCore/wtf/Atomics.h:109
> > +inline int atomicDecrement(int volatile* addend) { return __atomic_dec(addend); }
> 
> So __atomic_inc/dec() are functionally identical to android_atomic_inc/dec() removed in r94191, but are in the NDK, right?

Yes.

> 
> > Source/JavaScriptCore/wtf/Platform.h:306
> > +/* OS(ANDRO67299ID) - Android */
> 
> ??!!

Ugh, fixed.

[Adam Barth]

Comment on attachment 105805 [details]
Patch

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

The Platform.h changes look fine to me.

> Source/JavaScriptCore/wtf/Platform.h:698
>  #if !OS(WINDOWS) && !OS(SOLARIS) && !OS(QNX) \
>      && !OS(SYMBIAN) && !OS(HAIKU) && !OS(RVCT) \
> -    && !PLATFORM(BREWMP)
> +    && !OS(ANDROID) && !PLATFORM(BREWMP)

This should probably be a whitelist, but that's a change for another day.

[WebKit Review Bot]

Comment on attachment 105805 [details]
Patch

Clearing flags on attachment: 105805

Committed r94235: <http://trac.webkit.org/changeset/94235>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.