On Wed, Jun 10, 2026 at 07:38:12PM +0530, [email protected] wrote: > Builtins resolve in the frontend to a single internal function, > > IFN_ATOMIC_FETCH_MINMAX (ptr, value, memorder, is_min, zero_of_datatype)
Why do you need zero_of_datatype, when you have the value argument? Sure, I know pointer casts are useless, so you can't take the type from ptr, but why can't you take the type from the value argument? Make sure c-common.cc casts the value argument to TREE_TYPE (ptr), then the type should be ok until lowering or expansion. Also, I'd prefer memorder argument to come last, so ptr, value, is_min, memorder. > Signed-off-by: Soumya AR <[email protected]> This line belongs after ChangeLog entry, not before it. I don't see documentation, the patch should contain gcc/doc/extend.texi changes documenting the builtins, e.g. after __atomic_is_lock_free or so. > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -1421,6 +1421,7 @@ OBJS = \ > alias.o \ > alloc-pool.o \ > asm-toplevel.o \ > + atomic-ifn-lowering.o \ I'd prefer not to have a full new pass for this (even when it has a cfun-> guard), but will defer to Richi about this. pass_gimple_isel is supposedly too late, pass_optimize_widening_mul is -O1+ and non--Og only. > + /* Avoid lowering atomic IFN calls during the BB walk because lowering > + can split basic blocks and corrupt the iterator. Collect the calls > + and their matching descriptors first, then lower them one by one. */ > + auto_vec<std::pair<gcall *, const atomic_op_lowering *> > to_lower; > + FOR_EACH_BB_FN (bb, cfun) > + for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); > + gsi_next (&gsi)) > + { > + gcall *call = dyn_cast<gcall *> (gsi_stmt (gsi)); > + if (!call) > + continue; As you only handle internal calls, would be useful to continue whenever !gimple_call_internal_p. > + for (const atomic_op_lowering &op : atomic_op_table) > + if (gimple_call_internal_p (call, op.ifn)) > + { > + to_lower.safe_push ({ call, &op }); > + break; > + } Why? Many passes manage this to handle it normally in the walk. Pass &gsi to the lowering function and if changes are made, ensure to set it to the stmt in the exit bb (if any). If needed, arrange for gsi_next (&gsi) not to be done in that case. > + /* sync_resolve_params expects a sized builtin decl; we only > + register the generic form and lower via IFN, therefore, validate > + params manually. */ > + const char *err = NULL; > + if (params->length () < 3) > + err = "too few arguments to function %qE"; > + else if (params->length () > 3) > + err = "too many arguments to function %qE"; > + else if (!INTEGRAL_TYPE_P (datatype)) > + err = "argument 1 of %qE must point to an integer type"; > + else if (!INTEGRAL_TYPE_P (TREE_TYPE ((*params)[1]))) > + err = "argument 2 of %qE must be an integer type"; > + else if (!INTEGRAL_TYPE_P (TREE_TYPE ((*params)[2]))) > + err = "non-integer memory model argument 3 of %qE"; > + if (err) > + { > + if (complain) > + error_at (loc, err, function); > + return error_mark_node; > + } Don't do this, it is translation unfriendly. Either mark them all with G_("..."); or better just use if (complain) error_at; return error_mark_node; in each of those cases separately. > + /* Atomics can throw under -fnon-call-exceptions; mark the call > + NOTHROW otherwise. */ > + if (!flag_non_call_exceptions) > + TREE_NOTHROW (ret) = 1; Do you handle -fnon-call-exceptions during lowering? How do you handle it when expanded using hw insns? > +/* Expand atomic fetch minmax. */ > + > +static void > +expand_ATOMIC_FETCH_MINMAX (internal_fn, gcall *) > +{ > + /* Implement this. */ 2 spaces rather than tab. > +/* Each function must explicitly convert its argument to (int) (type of > + global_val) before passing it to ATOMIC_FETCH_MINMAX. The four functions > + each produce one such cast, so we expect exactly 4 in total. */ > +/* { dg-final { scan-tree-dump-times "= \\(int\\) x" 4 "gimple" } } */ > \ No newline at end of file Please avoid this. > @@ -328,7 +438,6 @@ test_or_fetch () > abort (); > } > > - > /* Test the OP routines with a result which isn't used. Use both variations > within each function. */ > Why (several times)? Jakub
