Bug 234333 - [libpas] Implement atomics in inline assembly if compiler is using ARM64
Summary: [libpas] Implement atomics in inline assembly if compiler is using ARM64
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-12-14 19:50 PST by Yusuke Suzuki
Modified: 2021-12-15 11:47 PST (History)
2 users (show)

See Also:


Attachments
Patch (23.35 KB, patch)
2021-12-14 19:57 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (23.35 KB, patch)
2021-12-14 20:16 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (23.35 KB, patch)
2021-12-15 04:36 PST, Yusuke Suzuki
fpizlo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2021-12-14 19:50:43 PST
[libpas] Implement atomics in inline assembly if compiler is using ARM64
Comment 1 Yusuke Suzuki 2021-12-14 19:57:11 PST
Created attachment 447194 [details]
Patch
Comment 2 Yusuke Suzuki 2021-12-14 20:16:50 PST
Created attachment 447197 [details]
Patch
Comment 3 Yusuke Suzuki 2021-12-15 04:36:42 PST
Created attachment 447222 [details]
Patch
Comment 4 Filip Pizlo 2021-12-15 10:02:07 PST
Comment on attachment 447222 [details]
Patch

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

I'm not sure how much I care, but it would be nice if store_atomic was a pas_utils thing.

> Source/bmalloc/libpas/src/libpas/pas_lock.h:116
>  #if PAS_COMPILER(CLANG)
> +#if PAS_COMPILER(CLANG_ARM64_ATOMICS_DEPENDENCY)
> +    asm volatile (
> +        "stlrb wzr, [%x[ptr]]\t\n"
> +        /* outputs */  :
> +        /* inputs  */  : [ptr]"r"(&lock->lock)
> +        /* clobbers */ : "memory"
> +    );
> +#else
>      __c11_atomic_store((_Atomic bool*)&lock->lock, false, __ATOMIC_SEQ_CST);
> +#endif
>  #else
>      __atomic_store_n((bool*)&lock->lock, false, __ATOMIC_SEQ_CST);
>  #endif

I think it would be way better if this atomic store thing was factored out into pas_utils.h.
Comment 5 Yusuke Suzuki 2021-12-15 11:29:18 PST
Comment on attachment 447222 [details]
Patch

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

>> Source/bmalloc/libpas/src/libpas/pas_lock.h:116
>>  #endif
> 
> I think it would be way better if this atomic store thing was factored out into pas_utils.h.

OK! Extracted atomic part.
Comment 6 Yusuke Suzuki 2021-12-15 11:46:46 PST
Committed r287092 (245284@trunk): <https://commits.webkit.org/245284@trunk>
Comment 7 Radar WebKit Bug Importer 2021-12-15 11:47:41 PST
<rdar://problem/86535186>