13/02/2023 17:36, Raslan Darawsheh: > From: Morten Brørup <m...@smartsharesystems.com> > > From: Bruce Richardson [mailto:bruce.richard...@intel.com] > > > 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> [...] > > > 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> > Tested-by: Raslan Darawsheh <rasl...@nvidia.com>
Applied, thanks.
