Bug 182610 – [MIPS] JSC needs to be built with -latomic

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

[Guillaume Emont]

Since r228149, on MIPS we need to link with -latomic, since __atomic_fetch_add_8 is not available as a compiler intrinsic.

[Guillaume Emont]

Created attachment 333412 [details]
Patch

Patch fixing this.

[Zan Dobersek]

Comment on attachment 333412 [details]
Patch

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

> Source/JavaScriptCore/CMakeLists.txt:125
> +if (WTF_CPU_MIPS)
> +    list(APPEND JavaScriptCore_LIBRARIES
> +        -latomic
> +    )
> +endif ()

Similar comment to what's in the ChangeLog would be nice here as well.

[Adrian Perez]

Comment on attachment 333412 [details]
Patch

In Source/WebKit/CMakeLists.txt, after line 780, there's a check
that tries to compile a snippet which uses “std::atomic<int64_t>”
and determines whether using libatomic is needed or not. Instead
of unconditionally adding the library as your patch does, could
you please change the CMake code there so it adds it conditionally?

The resulting snippet patch should be something like:

        if (ATOMIC_INT64_REQUIRES_LIBATOMIC)
            list(APPEND WebKit_LIBRARIES atomic)
   +        list(APPEND JavaScriptCore_LIBRARIES atomic)
        endif ()

Thanks!

[Adrian Perez]

(In reply to Adrian Perez from comment #3)
> Comment on attachment 333412 [details]
> Patch
> 
> In Source/WebKit/CMakeLists.txt, after line 780, there's a check
> that tries to compile a snippet which uses “std::atomic<int64_t>”
> and determines whether using libatomic is needed or not. Instead
> of unconditionally adding the library as your patch does, could
> you please change the CMake code there so it adds it conditionally?
> 
> The resulting snippet patch should be something like:
> 
>         if (ATOMIC_INT64_REQUIRES_LIBATOMIC)
>             list(APPEND WebKit_LIBRARIES atomic)
>    +        list(APPEND JavaScriptCore_LIBRARIES atomic)
>         endif ()
> 
> Thanks!

Correction: This is in the CMakeLists.txt for *WebKit*, so it would
be better to move the check to one of the top-level CMakeLists.txt
and then in Source/JavaScriptCore/CMakeLists.txt append the library
conditionally to the list.

[Guillaume Emont]

Created attachment 333495 [details]
Patch

New patch addressing Zan's comments.

[Guillaume Emont]

Note that this is to fix the build quickly. A more correct solution will be developed in bug#182622.

[WebKit Commit Bot]

Comment on attachment 333495 [details]
Patch

Clearing flags on attachment: 333495

Committed r228370: <https://trac.webkit.org/changeset/228370>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/37451082>