On Tue, Nov 09, 2021 at 07:37:48AM -0800, Andrii Nakryiko wrote: > On Mon, Nov 8, 2021 at 7:38 PM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > On Sun, Nov 07, 2021 at 10:13:11PM -0800, Andrii Nakryiko wrote: > > > +#define btf_dump__new(a1, a2, a3, a4) __builtin_choose_expr( \ > > > + __builtin_types_compatible_p(typeof(a4), btf_dump_printf_fn_t) + \ > > > + __builtin_types_compatible_p(typeof(a4), \ > > > + void(void *, const char *, va_list)), \ > > > + btf_dump__new_deprecated((void *)a1, (void *)a2, (void *)a3, (void *)a4),\ > > > + btf_dump__new((void *)a1, (void *)a2, (void *)a3, (void *)a4)) > > > > why '+' in the above? The return type of __builtin_types_compatible_p() is bool. > > What is bool + bool ? > > It suppose to be logical 'OR', right? > > __builtin_types_compatible_p() is defined as returning 0 or 1 (not > true/false). And __builtin_choose_expr() is also defined as comparing > first argument against 0, not as true/false. But in practice it > doesn't matter because bool is converted to 0 or 1 in arithmetic > operations. So I can switch to || with no effect. Let me know if you > still prefer logical || and I'll change. > > But yes to your last question, it's logical OR. Interesting. Looking at LLVM code it does indeed returns 'int'. At least typeof(_builtin_types_compatible_p(..)) seems to be 'int'. At the same type LLVM tests are using this macro: #define check_same_type(type1, type2) __builtin_types_compatible_p(type1, type2) && __builtin_types_compatible_p(type1 *, type2 *) While kernel has this macro: #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) Guessing that extra typeof() may resolve the difference between fn and &fn ? Not sure why LLVM took that approach. Anyway it will be removed once we hit 1.0, so no need to dig too deep. I think changing + to || is still worth doing. > > > > Maybe checking for ops type would be more robust ? > > opts can be NULL. At which point it's actually only compatible with > void *. Assuming that fn pointer in btf_dump__new_deprecated() will never be NULL?
