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.
2026-07-08 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/c-family/c-common.cc.jj 2026-07-08 11:09:58.407725160 +0200
+++ gcc/c-family/c-common.cc 2026-07-08 11:46:45.352615236 +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,
@@ -8582,21 +8654,39 @@ 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)
+ {
+ func_call = build2 (MODIFY_EXPR, void_type_node, old, func_call);
+ tree cmp = build2 (EQ_EXPR, boolean_type_node, old, 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);
Jakub
