Bug 143754 - [W32] weakCompareAndSwap assembler code is not used when building with MinGW GCC
Summary: [W32] weakCompareAndSwap assembler code is not used when building with MinGW GCC
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 133028
  Show dependency treegraph
 
Reported: 2015-04-15 06:13 PDT by LRN
Modified: 2015-11-17 02:34 PST (History)
6 users (show)

See Also:


Attachments
[GTK][W32] Allow MinGW-GCC to use GCC asm code for weakCompareAndSwap (2.10 KB, patch)
2015-04-15 08:28 PDT, LRN
ossy: review-
Details | Formatted Diff | Diff
Allow MinGW-GCC to use GCC asm code for weakCompareAndSwap (1.92 KB, patch)
2015-04-15 09:39 PDT, LRN
no flags Details | Formatted Diff | Diff
Allow MinGW-GCC to use GCC asm code for weakCompareAndSwap (3.70 KB, patch)
2015-04-23 08:34 PDT, LRN
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description LRN 2015-04-15 06:13:12 PDT
The ifdef says OS(WINDOWS), but means COMPILER(MSVC)
Comment 1 LRN 2015-04-15 08:28:39 PDT
Created attachment 250789 [details]
[GTK][W32] Allow MinGW-GCC to use GCC asm code for weakCompareAndSwap

OS(WINDOWS) != COMPILER(MSVC)
Comment 2 Csaba Osztrogonác 2015-04-15 09:14:47 PDT
Comment on attachment 250789 [details]
[GTK][W32] Allow MinGW-GCC to use GCC asm code for weakCompareAndSwap

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

> Source/JavaScriptCore/ChangeLog:8
> +        [W32] Fixup dummy HeapStatistics implementation
> +
> +        GCC warns that exitWithFailure is marked as noreturn, but it does return:
> +        ../webkitgtk-2.4.8/Source/JavaScriptCore/heap/HeapStatistics.cpp:135:1: warning: 'noreturn' function does return
> +
> +        Call exit(-1) to indicate that no, there is no returning from here.

It is unrelated to this patch.

> Source/JavaScriptCore/ChangeLog:16
> +        * heap/HeapStatistics.cpp:
> +        (JSC::HeapStatistics::exitWithFailure):

ditto

> Source/WTF/wtf/Atomics.h:263
> -#if !OS(WINDOWS) && (CPU(X86) || CPU(X86_64))
> +#if (!OS(WINDOWS) || COMPILER(GCC)) && (CPU(X86) || CPU(X86_64))

I think OS(WINDOWS) is very confusing and isn't good at all,
COMPILER(GCC) && (CPU(X86) || CPU(X86_64)) would be better.
Comment 3 LRN 2015-04-15 09:39:46 PDT
Created attachment 250797 [details]
Allow MinGW-GCC to use GCC asm code for weakCompareAndSwap

OS(WINDOWS) != COMPILER(MSVC)
Use COMPILER(GCC) as suggested.
Comment 4 Csaba Osztrogonác 2015-04-15 09:47:30 PDT
Comment on attachment 250797 [details]
Allow MinGW-GCC to use GCC asm code for weakCompareAndSwap

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

> Source/WTF/ChangeLog:12
> +        Allow MinGW-GCC to use GCC asm code for weakCompareAndSwap
> +
> +        OS(WINDOWS) != COMPILER(MSVC)
> +        Use COMPILER(GCC) as suggested.
> +
> +        [W32] weakCompareAndSwap assembler code is not used when building with MinGW GCC
> +        https://bugs.webkit.org/show_bug.cgi?id=143754
> +
> +        Reviewed by NOBODY (OOPS!).
> +

[W32] weakCompareAndSwap assembler code is not used when building with MinGW GCC
https://bugs.webkit.org/show_bug.cgi?id=143754

Reviewed by NOBODY (OOPS!).

... everyting else ...
Comment 5 LRN 2015-04-23 08:34:26 PDT
Created attachment 251436 [details]
Allow MinGW-GCC to use GCC asm code for weakCompareAndSwap

This version does so for *all* variants of this functions,
not just the one that needs it to compile.
Also uses saner conditions and explains the ones that are not as sane.

The code compiles and runs with this, but i can't be sure that i was able
to test all the cases where this function is used.
Comment 6 Carlos Garcia Campos 2015-05-18 23:51:10 PDT
Committed to 2.4 http://trac.webkit.org/changeset/184552

I've committed this unreviewed to 2.4 since it's needed to fix the win32 build, but for trunk someone should review this.
Comment 7 Csaba Osztrogonác 2015-09-24 06:52:27 PDT
After https://trac.webkit.org/changeset/190103 we don't need assembly CAS anymore.
Comment 8 Csaba Osztrogonác 2015-11-17 02:34:55 PST
Comment on attachment 251436 [details]
Allow MinGW-GCC to use GCC asm code for weakCompareAndSwap

Cleared review? from attachment 251436 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).