> From: Bruce Richardson [mailto:bruce.richard...@intel.com] > Sent: Monday, 13 February 2023 15.55 > > On Mon, Feb 13, 2023 at 03:44:55PM +0100, David Marchand wrote: > > Venerable RHEL7 clang 3.4.2 has (at least) two issues with lock > > annotations. > > > > A first one with regards to the attribute position: > > ../lib/vhost/vhost.h:518:2: error: GCC does not allow > > assert_exclusive_lock attribute in this position on a > > function definition [-Werror,-Wgcc-compat] > > __rte_assert_exclusive_lock(&vq->access_lock) > > ^ > > ../lib/eal/include/rte_lock_annotations.h:29:38: note: expanded > > from macro '__rte_assert_exclusive_lock' > > __attribute__((assert_exclusive_lock(__VA_ARGS__))) > > ^ > > > > This can be worked around by splitting and having the allocation on > the > > function declaration. > > > > But on the other hand, clang 3.4.2 does not seem to propagate those > > annotations in presence of a __builtin_expect (i.e. unlikely()), like > > for example when calling if (unlikely(rte_spinlock_trylock() == 0)). > > > > Those annotations were only working with clang in any case, so > restrict > > to clang versions newer than 3.5.0. > > > > Fixes: 657a98f38940 ("eal: annotate spinlock, rwlock and seqlock") > > > > Signed-off-by: David Marchand <david.march...@redhat.com> > > --- > > drivers/meson.build | 2 +- > > lib/meson.build | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/meson.build b/drivers/meson.build > > index bddc4a6cc4..0618c31a69 100644 > > --- a/drivers/meson.build > > +++ b/drivers/meson.build > > @@ -168,7 +168,7 @@ foreach subpath:subdirs > > enabled_drivers += name > > lib_name = '_'.join(['rte', class, name]) > > cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + > '.'.join([log_prefix, name]) > > - if annotate_locks and cc.has_argument('-Wthread-safety') > > + if annotate_locks and cc.get_id() == 'clang' and > cc.version().version_compare('>=3.5.0') > > cflags += '-DRTE_ANNOTATE_LOCKS' > > cflags += '-Wthread-safety' > > endif > > Are we likely to see any issues with this with any other compilers? > Should > we look to do a built-test in meson to determine feature support rather > than checking clang versions explicitly? > > On the plus side, checking clang version like this makes it clear when > we > can drop the conditional.
I prefer this over auto-detection in meson, for the reason mentioned by Bruce. Furthermore, different compilers may use different syntax in the code, so it is impossible to detect generically; the auto-detection would be per compiler anyway. Acked-by: Morten Brørup <m...@smartsharesystems.com>
