On Wed, Jul 08, 2026 at 11:54:31AM +0200, Jakub Jelinek wrote:
On Wed, Jul 08, 2026 at 10:25:30AM +0200, Jakub Jelinek wrote:
I'm afraid this approach is incorrect.
The problem is that in GIMPLE pointer casts are useless, so what exact
pointee you get is quite random, and it can be completely unrelated type.
If one e.g. does
void
foo (unsigned _BitInt(17) *p, bool x)
{
if (x)
__atomic_add_fetch (p, 0x0fff0, 0);
else
__atomic_add_fetch ((unsigned long *) p, 0x0fff0, 0);
}
you can get unsigned _BitInt(17) "incorrectly" in both cases, while
void
bar (unsigned long *p, bool x)
{
if (x)
__atomic_add_fetch ((unsigned _BitInt(17) *) p, 0x0fff0, 0);
else
__atomic_add_fetch (p, 0x0fff0, 0);
}
in neither, etc.
IMNSHO, if unsigned _BitInt(17) * pointer makes it to say __atomic_add_fetch_4,
then it should be irrelevant, the required operation is still to load
full 32 bits from the pointer, add another 32 bit value to it and store
full 32 bits.
IMNSHO this is solely about some of the type-generic atomic/sync builtins
(those documented to take TYPE * arguments) and needs to be handled in the
FE, likely in gcc/c-family/c-common.cc (resolve_overloaded_builtin).
There already is code to transform various type-generic builtins into a CAS
loop for say unsigned _BitInt(253), so I think it should be used also for
the case where the type is BITINT_TYPE with any padding bits in the
extend other than bitint_ext_undef mode.
Here is a completely untested patch for that, but guess the test
I've posted in the last mail needs to be turned into a proper executable
testcase and verify (e.g. using bitintext.h macros) whether the extension
was right.
I've added a testcase and that revealed a bug in the patch, in the
__sync_fetch_and_* cases the patch returned the new value anyway like
in the __sync_*_and_fetch case.
The following patch fixes that.
So far tested with a cross to riscv64-linux and running the test on cfarm94.
Ok for trunk if this passes bootstrap/regtest on x86_64-linux and
i686-linux?
2026-07-09 Jakub Jelinek <[email protected]>
PR target/124948
* c-common.cc (sync_resolve_size): Return -1 for fetch ops
on _BitInt types with padding bits where the target requires
extension into the padding bits.
(atomic_bitint_fetch_using_cas_loop): Handle also __sync_*
fetch builtins.
* gcc.dg/torture/bitint-100.c: New test.
--- gcc/c-family/c-common.cc.jj 2026-07-08 11:09:58.407725160 +0200
+++ gcc/c-family/c-common.cc 2026-07-09 09:48:53.446148261 +0200
@@ -7746,7 +7746,23 @@ sync_resolve_size (tree function, vec<tr
}
if (size == 1 || size == 2 || size == 4 || size == 8 || size == 16)
- return size;
+ {
+ /* For _BitInt with padding bits where the ABI mandates sign or
+ zero extension into the padding bits, force a CAS loop so that
+ the extension is properly performed. */
+ if (fetch
+ && BITINT_TYPE_P (type)
+ && (TYPE_PRECISION (type)
+ != GET_MODE_PRECISION (SCALAR_TYPE_MODE (type))))
+ {
+ struct bitint_info info;
+ bool ok = targetm.c.bitint_type_info (TYPE_PRECISION (type), &info);
+ gcc_assert (ok);
+ if (info.extended != bitint_ext_undef)
+ return -1;
+ }
+ return size;
+ }
if (fetch && !orig_format && BITINT_TYPE_P (type))
return -1;
@@ -8405,7 +8421,8 @@ resolve_overloaded_atomic_store (locatio
}
/* Emit __atomic*fetch* on _BitInt which doesn't have a size of
- 1, 2, 4, 8 or 16 bytes using __atomic_compare_exchange loop.
+ 1, 2, 4, 8 or 16 bytes (or if it has padding bits and they
+ need to be extended) using __atomic_compare_exchange loop.
ORIG_CODE is the DECL_FUNCTION_CODE of ORIG_FUNCTION and
ORIG_PARAMS arguments of the call. */
@@ -8417,6 +8434,7 @@ atomic_bitint_fetch_using_cas_loop (loca
{
enum tree_code code = ERROR_MARK;
bool return_old_p = false;
+ bool sync_p = false;
switch (orig_code)
{
case BUILT_IN_ATOMIC_ADD_FETCH_N:
@@ -8459,13 +8477,65 @@ atomic_bitint_fetch_using_cas_loop (loca
code = BIT_IOR_EXPR;
return_old_p = true;
break;
+ case BUILT_IN_SYNC_ADD_AND_FETCH_N:
+ code = PLUS_EXPR;
+ sync_p = true;
+ break;
+ case BUILT_IN_SYNC_SUB_AND_FETCH_N:
+ code = MINUS_EXPR;
+ sync_p = true;
+ break;
+ case BUILT_IN_SYNC_OR_AND_FETCH_N:
+ code = BIT_IOR_EXPR;
+ sync_p = true;
+ break;
+ case BUILT_IN_SYNC_AND_AND_FETCH_N:
+ code = BIT_AND_EXPR;
+ sync_p = true;
+ break;
+ case BUILT_IN_SYNC_XOR_AND_FETCH_N:
+ code = BIT_XOR_EXPR;
+ sync_p = true;
+ break;
+ case BUILT_IN_SYNC_NAND_AND_FETCH_N:
+ sync_p = true;
+ break;
+ case BUILT_IN_SYNC_FETCH_AND_ADD_N:
+ code = PLUS_EXPR;
+ return_old_p = true;
+ sync_p = true;
+ break;
+ case BUILT_IN_SYNC_FETCH_AND_SUB_N:
+ code = MINUS_EXPR;
+ return_old_p = true;
+ sync_p = true;
+ break;
+ case BUILT_IN_SYNC_FETCH_AND_OR_N:
+ code = BIT_IOR_EXPR;
+ return_old_p = true;
+ sync_p = true;
+ break;
+ case BUILT_IN_SYNC_FETCH_AND_AND_N:
+ code = BIT_AND_EXPR;
+ return_old_p = true;
+ sync_p = true;
+ break;
+ case BUILT_IN_SYNC_FETCH_AND_XOR_N:
+ code = BIT_XOR_EXPR;
+ return_old_p = true;
+ sync_p = true;
+ break;
+ case BUILT_IN_SYNC_FETCH_AND_NAND_N:
+ return_old_p = true;
+ sync_p = true;
+ break;
default:
gcc_unreachable ();
}
- if (orig_params->length () != 3)
+ if (orig_params->length () != (sync_p ? 2 : 3))
{
- if (orig_params->length () < 3)
+ if (orig_params->length () < (sync_p ? 2 : 3))
error_at (loc, "too few arguments to function %qE", orig_function);
else
error_at (loc, "too many arguments to function %qE", orig_function);
@@ -8480,12 +8550,14 @@ atomic_bitint_fetch_using_cas_loop (loca
tree lhs_addr = (*orig_params)[0];
tree val = convert (nonatomic_lhs_type, (*orig_params)[1]);
- tree model = convert (integer_type_node, (*orig_params)[2]);
+ tree model
+ = sync_p ? NULL_TREE : convert (integer_type_node, (*orig_params)[2]);
if (!c_dialect_cxx ())
{
lhs_addr = c_fully_fold (lhs_addr, false, NULL);
val = c_fully_fold (val, false, NULL);
- model = c_fully_fold (model, false, NULL);
+ if (model)
+ model = c_fully_fold (model, false, NULL);
}
if (TREE_SIDE_EFFECTS (lhs_addr))
{
@@ -8501,7 +8573,7 @@ atomic_bitint_fetch_using_cas_loop (loca
NULL_TREE);
add_stmt (val);
}
- if (TREE_SIDE_EFFECTS (model))
+ if (model && TREE_SIDE_EFFECTS (model))
{
tree var = create_tmp_var_raw (integer_type_node);
model = build4 (TARGET_EXPR, integer_type_node, var, model, NULL_TREE,
@@ -8519,6 +8591,8 @@ atomic_bitint_fetch_using_cas_loop (loca
TREE_ADDRESSABLE (newval) = 1;
suppress_warning (newval);
+ tree retval = NULL_TREE;
+
tree loop_decl = create_artificial_label (loc);
tree loop_label = build1 (LABEL_EXPR, void_type_node, loop_decl);
@@ -8582,21 +8656,43 @@ atomic_bitint_fetch_using_cas_loop (loca
/* if (__atomic_compare_exchange (addr, &old, &new, false, model, model))
goto done; */
- fndecl = builtin_decl_explicit (BUILT_IN_ATOMIC_COMPARE_EXCHANGE);
+ if (sync_p)
+ fndecl = builtin_decl_explicit (BUILT_IN_SYNC_VAL_COMPARE_AND_SWAP_N);
+ else
+ fndecl = builtin_decl_explicit (BUILT_IN_ATOMIC_COMPARE_EXCHANGE);
params->quick_push (lhs_addr);
- params->quick_push (old_addr);
- params->quick_push (newval_addr);
- params->quick_push (integer_zero_node);
- params->quick_push (model);
- if (tree_fits_uhwi_p (model)
- && (tree_to_uhwi (model) == MEMMODEL_RELEASE
- || tree_to_uhwi (model) == MEMMODEL_ACQ_REL))
- params->quick_push (build_int_cst (integer_type_node, MEMMODEL_RELAXED));
+ if (sync_p)
+ {
+ params->quick_push (old);
+ params->quick_push (newval);
+ }
else
- params->quick_push (model);
+ {
+ params->quick_push (old_addr);
+ params->quick_push (newval_addr);
+ params->quick_push (integer_zero_node);
+ params->quick_push (model);
+ if (tree_fits_uhwi_p (model)
+ && (tree_to_uhwi (model) == MEMMODEL_RELEASE
+ || tree_to_uhwi (model) == MEMMODEL_ACQ_REL))
+ params->quick_push (build_int_cst (integer_type_node,
+ MEMMODEL_RELAXED));
+ else
+ params->quick_push (model);
+ }
func_call = resolve_overloaded_builtin (loc, fndecl, params);
if (func_call == NULL_TREE)
func_call = build_function_call_vec (loc, vNULL, fndecl, params, NULL);
+ if (sync_p)
+ {
+ if (return_old_p)
+ retval = create_tmp_var_raw (nonatomic_lhs_type);
+ else
+ retval = old;
+ func_call = build2 (MODIFY_EXPR, void_type_node, retval, func_call);
+ tree cmp = build2 (EQ_EXPR, boolean_type_node, retval, newval);
+ func_call = build2 (COMPOUND_EXPR, boolean_type_node, func_call, cmp);
+ }
tree goto_stmt = build1 (GOTO_EXPR, void_type_node, done_decl);
SET_EXPR_LOCATION (goto_stmt, loc);
@@ -8606,6 +8702,12 @@ atomic_bitint_fetch_using_cas_loop (loca
SET_EXPR_LOCATION (stmt, loc);
add_stmt (stmt);
+ if (sync_p && return_old_p)
+ {
+ stmt = build2_loc (loc, MODIFY_EXPR, void_type_node, old, retval);
+ add_stmt (stmt);
+ }
+
/* goto loop; */
goto_stmt = build1 (GOTO_EXPR, void_type_node, loop_decl);
SET_EXPR_LOCATION (goto_stmt, loc);
--- gcc/testsuite/gcc.dg/torture/bitint-100.c.jj 2026-07-09
08:25:06.764615958 +0200
+++ gcc/testsuite/gcc.dg/torture/bitint-100.c 2026-07-09 09:19:20.063962240
+0200
@@ -0,0 +1,197 @@
+/* PR target/124948 */
+/* { dg-do run { target bitint } } */
+/* { dg-options "-std=c23 -pedantic-errors" } */
+/* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
+/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
+
+_BitInt(17) a;
+unsigned _BitInt(17) b;
+
+#include "../bitintext.h"
+
+[[gnu::noipa]] _BitInt(17)
+f1 (_BitInt(17) *p, _BitInt(32) q)
+{
+ return __atomic_add_fetch (p, q, __ATOMIC_RELAXED);
+}
+
+[[gnu::noipa]] _BitInt(17)
+f2 (_BitInt(17) *p, _BitInt(32) q)
+{
+ return __atomic_sub_fetch (p, q, __ATOMIC_RELAXED);
+}
+
+[[gnu::noipa]] _BitInt(17)
+f3 (_BitInt(17) *p, _BitInt(32) q)
+{
+ return __atomic_fetch_add (p, q, __ATOMIC_RELAXED);
+}
+
+[[gnu::noipa]] _BitInt(17)
+f4 (_BitInt(17) *p, _BitInt(32) q)
+{
+ return __atomic_fetch_sub (p, q, __ATOMIC_RELAXED);
+}
+
+[[gnu::noipa]] _BitInt(17)
+f5 (_BitInt(17) *p, _BitInt(32) q)
+{
+ return __sync_add_and_fetch (p, q);
+}
+
+[[gnu::noipa]] _BitInt(17)
+f6 (_BitInt(17) *p, _BitInt(32) q)
+{
+ return __sync_sub_and_fetch (p, q);
+}
+
+[[gnu::noipa]] _BitInt(17)
+f7 (_BitInt(17) *p, _BitInt(32) q)
+{
+ return __sync_fetch_and_add (p, q);
+}
+
+[[gnu::noipa]] _BitInt(17)
+f8 (_BitInt(17) *p, _BitInt(32) q)
+{
+ return __sync_fetch_and_sub (p, q);
+}
+
+[[gnu::noipa]] unsigned _BitInt(17)
+f9 (unsigned _BitInt(17) *p, unsigned _BitInt(32) q)
+{
+ return __atomic_add_fetch (p, q, __ATOMIC_RELAXED);
+}
+
+[[gnu::noipa]] unsigned _BitInt(17)
+f10 (unsigned _BitInt(17) *p, unsigned _BitInt(32) q)
+{
+ return __atomic_sub_fetch (p, q, __ATOMIC_RELAXED);
+}
+
+[[gnu::noipa]] unsigned _BitInt(17)
+f11 (unsigned _BitInt(17) *p, unsigned _BitInt(32) q)
+{
+ return __atomic_fetch_add (p, q, __ATOMIC_RELAXED);
+}
+
+[[gnu::noipa]] unsigned _BitInt(17)
+f12 (unsigned _BitInt(17) *p, unsigned _BitInt(32) q)
+{
+ return __atomic_fetch_sub (p, q, __ATOMIC_RELAXED);
+}
+
+[[gnu::noipa]] unsigned _BitInt(17)
+f13 (unsigned _BitInt(17) *p, unsigned _BitInt(32) q)
+{
+ return __sync_add_and_fetch (p, q);
+}
+
+[[gnu::noipa]] unsigned _BitInt(17)
+f14 (unsigned _BitInt(17) *p, unsigned _BitInt(32) q)
+{
+ return __sync_sub_and_fetch (p, q);
+}
+
+[[gnu::noipa]] unsigned _BitInt(17)
+f15 (unsigned _BitInt(17) *p, unsigned _BitInt(32) q)
+{
+ return __sync_fetch_and_add (p, q);
+}
+
+[[gnu::noipa]] unsigned _BitInt(17)
+f16 (unsigned _BitInt(17) *p, unsigned _BitInt(32) q)
+{
+ return __sync_fetch_and_sub (p, q);
+}
+
+int
+main ()
+{
+ _BitInt(17) c;
+ unsigned _BitInt(17) d;
+ a = 64295wb;
+ BEXTC (a);
+ c = f1 (&a, 187040987wb);
+ BEXTC (a);
+ BEXTC (c);
+ if (a != -65534wb || c != a)
+ __builtin_abort ();
+ c = f2 (&a, 394264674wb);
+ BEXTC (a);
+ BEXTC (c);
+ if (a != 65440wb || c != a)
+ __builtin_abort ();
+ c = f3 (&a, 840434595wb);
+ BEXTC (a);
+ BEXTC (c);
+ if (a != -64701wb || c != 65440wb)
+ __builtin_abort ();
+ c = f4 (&a, 591403122wb);
+ BEXTC (a);
+ BEXTC (c);
+ if (a != 60113wb || c != -64701wb)
+ __builtin_abort ();
+ c = f5 (&a, 1215571163wb);
+ BEXTC (a);
+ BEXTC (c);
+ if (a != -61524wb || c != a)
+ __builtin_abort ();
+ c = f6 (&a, 1913664021wb);
+ BEXTC (a);
+ BEXTC (c);
+ if (a != 56727wb || c != a)
+ __builtin_abort ();
+ c = f7 (&a, 858931586wb);
+ BEXTC (a);
+ BEXTC (c);
+ if (a != -57575wb || c != 56727wb)
+ __builtin_abort ();
+ c = f8 (&a, 286413601wb);
+ BEXTC (a);
+ BEXTC (c);
+ if (a != 52216wb || c != -57575wb)
+ __builtin_abort ();
+ b = 77593uwb;
+ BEXTC (b);
+ d = f9 (&b, 858861337uwb);
+ BEXTC (b);
+ BEXTC (d);
+ if (b != 24114uwb || d != b)
+ __builtin_abort ();
+ d = f10 (&b, 1431383831uwb);
+ BEXTC (b);
+ BEXTC (d);
+ if (b != 77595uwb || d != b)
+ __builtin_abort ();
+ d = f11 (&b, 305231735uwb);
+ BEXTC (b);
+ BEXTC (d);
+ if (b != 42642uwb || d != 77595uwb)
+ __builtin_abort ();
+ d = f12 (&b, 591497352uwb);
+ BEXTC (b);
+ BEXTC (d);
+ if (b != 73226uwb || d != 42642uwb)
+ __builtin_abort ();
+ d = f13 (&b, 1985058934uwb);
+ BEXTC (b);
+ BEXTC (d);
+ if (b != 46720uwb || d != b)
+ __builtin_abort ();
+ d = f14 (&b, 840018893uwb);
+ BEXTC (b);
+ BEXTC (d);
+ if (b != 68275uwb || d != b)
+ __builtin_abort ();
+ d = f15 (&b, 1698807006uwb);
+ BEXTC (b);
+ BEXTC (d);
+ if (b != 51089uwb || d != 68275uwb)
+ __builtin_abort ();
+ d = f16 (&b, 877788892uwb);
+ BEXTC (b);
+ BEXTC (d);
+ if (b != 51381uwb || d != 51089uwb)
+ __builtin_abort ();
+}
Jakub
