Verified on our riscv64-linux env as well, thanks :)
Jakub Jelinek <[email protected]> 於 2026年7月9日週四 下午4:00寫道： > > 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 >
