On Mon, Jan 19, 2026 at 12:19 AM Josef Melcr <[email protected]> wrote:
>
> Hi,
>
> On 1/16/26 3:46 AM, Andrew Pinski wrote:
>
> Was this ever reviewed or a new patch submitted here?
> If not, here is my stab at reviewing this. Also I think adding this
> will have to wait until stage 1 of GCC 17.
>
> No, this is the only version of the patch so far and you are the first to 
> review it :)
>
> I am not sure if we should mention clang attribute as being the
> counterpart to this; it might get confusing.
> As you mentioned they are not mutually compatible.
> Maybe just:
> This attribute is similar to the clang @code{callback} attribute but
> it is not compatible with it.
>
> Also I would move the comment about multiple times away from the
> differences of the clang attribute.
>
> Also since you have constraints on the attribute which would cause
> undefined behavior:
> "callback may not escape the translation unit of the annotated function" etc.
> It would be good to file a bug for the analyzer to detect each of
> those cases with an example of bad code and constraint it is
> violating.
> This way someone would go in and implement an analyzer code to detect them.
>
> Some rudimentary analysis is done in the attribute handler 
> (attr-callback.cc:192), although checks for addresses escaping (and other 
> constraints) are not implemented.  Do I file the bug now, or after the patch 
> is committed?  I'll implement the rest of your suggestions and send a v2.

Before or after is fine for the bug reports.

>
>
> As a side note, the patch series mentioned in the original email has already 
> been committed to master, so the dependencies of this patch are sorted out, 
> though I agree that this patch should wait for stage1 of GCC17.

I don't see any harm in waiting for stage 1 here. Since this won't be
used right away by folks.

Thanks,
Andrew Pinski

>
> Thanks,
> Andrew
>
> Thanks,
>
> Josef
>
> +
> +An example usage:
> +
> +@smallexample
> +[[gnu::callback_only(1, 3, 2)]]
> +void foo(void (*bar)(int*, double*), double* y, int* x);
> +@end smallexample
> +
> +means that there is a call @code{bar (x, y)} inside @code{foo}.
>
>  @cindex @code{gnu_inline} function attribute
>  @item gnu_inline
> diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
> index 2105c9a2ef7..94c952a3bbb 100644
> --- a/gcc/ipa-cp.cc
> +++ b/gcc/ipa-cp.cc
> @@ -6236,7 +6236,7 @@ purge_useless_callback_edges ()
>               if (dump_file)
>                 fprintf (dump_file, "\tExamining callbacks of edge %s -> 
> %s:\n",
>                          e->caller->dump_name (), e->callee->dump_name ());
> -             if (!lookup_attribute (CALLBACK_ATTR_IDENT,
> +             if (!lookup_attribute ("gnu", CALLBACK_ATTR_IDENT,
>                                      DECL_ATTRIBUTES (e->callee->decl))
>                   && !callback_is_special_cased (e->callee->decl, 
> e->call_stmt))
>                 {
> diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
> index 72a0f814fcd..21e1f9afcb1 100644
> --- a/gcc/ipa-prop.cc
> +++ b/gcc/ipa-prop.cc
> @@ -2550,11 +2550,11 @@ ipa_compute_jump_functions_for_edge (struct 
> ipa_func_body_info *fbi,
>                   /* Argument is a pointer to a function. Look for a callback
>                      attribute describing this argument.  */
>                   tree callback_attr
> -                   = lookup_attribute (CALLBACK_ATTR_IDENT,
> +                   = lookup_attribute ("gnu", CALLBACK_ATTR_IDENT,
>                                         DECL_ATTRIBUTES (cs->callee->decl));
>                   for (; callback_attr;
>                        callback_attr
> -                      = lookup_attribute (CALLBACK_ATTR_IDENT,
> +                      = lookup_attribute ("gnu", CALLBACK_ATTR_IDENT,
>                                            TREE_CHAIN (callback_attr)))
>                     if (callback_get_fn_index (callback_attr) == n)
>                       break;
> diff --git a/gcc/testsuite/gcc.dg/attr-callback.c 
> b/gcc/testsuite/gcc.dg/attr-callback.c
> new file mode 100755
> index 00000000000..4ebd8eefff2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/attr-callback.c
> @@ -0,0 +1,106 @@
> +/* Test callback attribute error checking. */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-std=gnu17 -Wattributes" } */
> +
> +[[gnu::callback_only(1, 2)]]
> +void
> +correct_1(void (*)(int*), int*);
> +
> +[[gnu::callback_only(1, 2, 3)]]
> +void
> +correct_2(void (*)(int*, double*), int*, double*);
> +
> +[[gnu::callback_only(1, 2, 3), gnu::callback_only(4, 5)]]
> +void
> +correct_3(void (*)(int*, double*), int*, double*, int (*)(void*), void*);
> +
> +[[gnu::callback_only(1, 0)]]
> +void
> +unknown_1(void (*)(int*));
> +
> +[[gnu::callback_only(1, 2, 0)]]
> +void
> +unknown_2(void (*)(int*, double*), int*, double*, char*);
> +
> +[[gnu::callback_only(1, 0, 3, 3)]]
> +void
> +too_many(void (*)(int*, double*), int*, double*); /* { dg-error "argument 
> number mismatch, 2 expected, got 3" }*/
> +
> +[[gnu::callback_only(1, 2)]]
> +void
> +too_few_1(void (*)(int*, double*), int*, double*); /* { dg-error "argument 
> number mismatch, 2 expected, got 1" }*/
> +
> +[[gnu::callback_only(1)]]
> +void
> +too_few_2(void (*)(int*, double*), int*, double*); /* { dg-error "argument 
> number mismatch, 2 expected, got 0" }*/
> +
> +[[gnu::callback_only(3, 1)]]
> +void
> +promotion(char*, float, int (*)(int*));
> +
> +[[gnu::callback_only(2, 3)]]
> +void
> +downcast(char*, void* (*)(float*), double*);
> +
> +[[gnu::callback_only(1, 2, 5)]]
> +void
> +out_of_range_1(char (*)(float*, double*), float*, double*, int*); /* { 
> dg-error "callback argument index 5 is out of range" } */
> +
> +[[gnu::callback_only(1, -2, 3)]]
> +void
> +out_of_range_2(char (*)(float*, double*), float*, double*, int*); /* { 
> dg-error "callback argument index -2 is out of range" } */
> +
> +[[gnu::callback_only(-1, 2, 3)]]
> +void
> +out_of_range_3(char (*)(float*, double*), float*, double*, int*); /* { 
> dg-error "callback function index -1 is out of range" } */
> +
> +[[gnu::callback_only(67, 2, 3)]]
> +void
> +out_of_range_4(char (*)(float*, double*), float*, double*, int*); /* { 
> dg-error "callback function index 67 is out of range" } */
> +
> +[[gnu::callback_only(0, 2, 3)]]
> +void
> +unknown_fn(char (*)(float*, double*), float*, double*, int*); /* { dg-error 
> "callback function position cannot be marked as unknown" } */
> +
> +[[gnu::callback_only(1, 2)]]
> +void
> +not_a_fn(int, int); /* { dg-error "argument no. 1 is not an address of a 
> function" } */
> +
> +struct S{
> +  int x;
> +};
> +
> +[[gnu::callback_only(1, 2)]]
> +void
> +incompatible_types_1(void (*)(struct S*), struct S); /* { dg-error "argument 
> type at index 2 is not compatible with callback argument type at index 1" } */
> +
> +[[gnu::callback_only(1, 3, 2)]]
> +void
> +incompatible_types_2(void (*)(struct S*, int*), int*, double); /* { dg-error 
> "argument type at index 3 is not compatible with callback argument type at 
> index 1" } */
> +
> +[[gnu::callback_only(1, "2")]]
> +void
> +wrong_arg_type_1(void (*)(void*), void*); /* { dg-error "argument no. 1 is 
> not an integer constant" } */
> +
> +[[gnu::callback_only("not a number", 2, 2)]]
> +void
> +wrong_arg_type_2(void (*)(void*, void*), void*); /* { dg-error "argument 
> specifying callback function position is not an integer constant" } */
> +
> +[[gnu::callback_only(1, 2), gnu::callback_only(1, 3)]]
> +void
> +multiple_single_fn(void (*)(int*), int*, int*); /* { dg-error "function 
> declaration has multiple callback attributes describing argument no. 1" } */
> +
> +/* Check that the attribute won't resolve outside of our namespace.  */
> +
> +[[callback_only(1, 2)]] /* { dg-warning "ignored" } */
> +void
> +ignore_1(void (*)(int*), int*);
> +
> +[[callback(1, 2)]] /* { dg-warning "ignored" } */
> +void
> +ignore_2(void (*)(int*), int*);
> +
> +[[gnu::callback(1, 2)]]
> +void
> +ignore_3(void (*)(int*), int*); /* { dg-warning "ignored" } */
> diff --git a/gcc/testsuite/gcc.dg/ipa/ipcp-cb2.c 
> b/gcc/testsuite/gcc.dg/ipa/ipcp-cb2.c
> new file mode 100644
> index 00000000000..f7ff9868b55
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/ipcp-cb2.c
> @@ -0,0 +1,51 @@
> +/* Test that we can handle multiple callback attributes and use them to
> +   propagate into callbacks. 'cb1' body borrowed from a ipa-cp test to get 
> the
> +   pass to work. */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fdump-ipa-cp" } */
> +
> +struct S {
> +  int a, b, c;
> +};
> +
> +extern void *blah(int, void *);
> +
> +[[gnu::callback_only(1, 2), gnu::callback_only(3, 4, 5)]] extern void
> +call(void (*fn1)(struct S *), struct S *a, void (*fn2)(struct S *, struct S 
> *),
> +     struct S *b, struct S *c);
> +
> +void cb1(struct S *p) {
> +  int i, c = p->c;
> +  int b = p->b;
> +  void *v = (void *)p;
> +
> +  for (i = 0; i < c; i++)
> +    v = blah(b + i, v);
> +}
> +
> +void cb2(struct S *a, struct S *b) {
> +  cb1(a);
> +  cb1(b);
> +}
> +
> +void test(int a, int b, int c) {
> +  struct S s;
> +  s.a = a;
> +  s.b = b;
> +  s.c = c;
> +  struct S ss;
> +  ss.a = s.c;
> +  ss.b = s.b;
> +  ss.c = s.a;
> +  call(cb1, &s, cb2, &s, &ss);
> +}
> +
> +int main() {
> +  test(1, 64, 32);
> +  return 0;
> +}
> +
> +/* { dg-final { scan-ipa-dump "Creating a specialized node of cb1" "cp" } } 
> */
> +/* { dg-final { scan-ipa-dump "Creating a specialized node of cb2" "cp" } } 
> */
> +/* { dg-final { scan-ipa-dump-times "Aggregate replacements: " 2 "cp" } } */
> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> index 145e758600e..c6428dced93 100644
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -102,7 +102,7 @@ struct die_struct;
>     meant to be used for the construction of builtin functions.  They were 
> only
>     added because Fortran uses them for attributes of builtins.  */
>
> -/* callback(1, 2) */
> +/* callback_only(1, 2) */
>  #define ECF_CB_1_2               (1 << 17)
>
>  /* Call argument flags.  */
> --
> 2.51.0
>

Reply via email to