On Tue, Nov 19, 2024 at 08:51:32AM +0100, Jakub Jelinek wrote:
> Hi!
>
> Only __atomic_* builtins are meant to work on arbitrary _BitInt types
> (if not supported in hw we emit a CAS loop which uses __atomic_load_*
> in that case), the compatibility __sync_* builtins work only if there
> is a corresponding normal integral type (for _BitInt on 32-bit ARM
> we'll need to limit even that to no padding, because the padding bits
> are well defined there and the hw or libatomic __sync_* APIs don't
> guarantee that), IMHO people shouldn't mix very old APIs with very
> new ones and I don't see a replacement for the __atomic_load_*.
>
> For size > 16 that is how it already correctly behaves,
> in the hunk shown in the patch it is immediately followed by
>
> if (fetch && !orig_format && TREE_CODE (type) == BITINT_TYPE)
> return -1;
>
> which returns -1 for the __atomic_* builtins (i.e. !orig_format),
> which causes caller to use atomic_bitint_fetch_using_cas_loop,
> and otherwise does diagnostic and return 0 (which causes caller
> to punt). But for size == 16 if TImode isn't suipported (i.e.
> mostly 32-bit arches), we return (correctly) -1 if !orig_format,
> so again force atomic_bitint_fetch_using_cas_loop on those arches
> for e.g. _BitInt(115), but for orig_format the function returns
> 16 as if it could do 16 byte __sync_*_and_* (which it can't
> because TImode isn't supported; for 16 byte it can only do
> (perhaps using libatomic) normal compare and swap). So we need
> to error and return 0, rather than return 16.
>
> The following patch ensures that.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Makes sense -- patch is OK, thanks.
> 2024-11-19 Jakub Jelinek <ja...@redhat.com>
>
> PR c/117641
> * c-common.cc (sync_resolve_size): For size == 16 fetch of
> BITINT_TYPE if TImode isn't supported scalar mode diagnose
> and return 0 if orig_format instead of returning 16.
>
> * gcc.dg/bitint-115.c: New test.
>
> --- gcc/c-family/c-common.cc.jj 2024-11-15 08:43:34.597097027 +0100
> +++ gcc/c-family/c-common.cc 2024-11-18 16:21:06.306266787 +0100
> @@ -7458,10 +7458,13 @@ sync_resolve_size (tree function, vec<tr
> size = tree_to_uhwi (TYPE_SIZE_UNIT (type));
> if (size == 16
> && fetch
> - && !orig_format
> && TREE_CODE (type) == BITINT_TYPE
> && !targetm.scalar_mode_supported_p (TImode))
> - return -1;
> + {
> + if (!orig_format)
> + return -1;
> + goto incompatible;
> + }
>
> if (size == 1 || size == 2 || size == 4 || size == 8 || size == 16)
> return size;
> --- gcc/testsuite/gcc.dg/bitint-115.c.jj 2024-11-18 16:41:14.023399087
> +0100
> +++ gcc/testsuite/gcc.dg/bitint-115.c 2024-11-18 16:42:38.515219036 +0100
> @@ -0,0 +1,9 @@
> +/* PR c/117641 */
> +/* { dg-do compile { target bitint575 } } */
> +/* { dg-options "-std=c23" } */
> +
> +void
> +foo (_BitInt(128) *b)
> +{
> + __sync_fetch_and_add (b, 1); /* { dg-error "incompatible" "" {
> target { ! int128 } } } */
> +}
>
> Jakub
>
Marek
