Re: dev Digest, Vol 195, Issue 45

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Wed, 16 May 2018 02:08:48 +0000
> From: Gavin Hu <Gavin.Hu@arm.com>
> To: "dev@dpdk.org" <dev@dpdk.org>, "Jacob,  Jerin"
>  <Jerin.JacobKollanukkaran@cavium.com>
> Subject: Re: [dpdk-dev] dev Digest, Vol 195, Issue 45
> 
> >
> > For ARM64 platform, libdpdk.a includes the
> > librte_pmd_octeontx_ssovf.a, which requires the libatomic.a
> > support.The atomic lib is built-in in the gcc toolchain, but for clang it has to be explicitly linked.
> > For more details, please refer to
> > https://clang.llvm.org/docs/Toolchain.html
> >
> > ~/dpdk/build/lib/librte_pmd_octeontx_ssovf.a(timvf_worker.o): In function `timvf_timer_cancel_burst':
> > timvf_worker.c:(.text+0x80): undefined reference to `__atomic_fetch_add_8'
> > /home/gavin/arm_repo/dpdk/build/lib/librte_pmd_octeontx_ssovf.a(timvf_worker.o): In function `timvf_timer_arm_burst_sp':
> > timvf_worker.c:(.text+0x200): undefined reference to `__atomic_fetch_add_8'
> > timvf_worker.c:(.text+0x244): undefined reference to `__atomic_store_2'
> > timvf_worker.c:(.text+0x278): undefined reference to `__atomic_fetch_add_4'
> > timvf_worker.c:(.text+0x30c): undefined reference to `__atomic_store_2'
> >
> > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> 
> 
> 
> Following patch is part of upstream. Are you testing with following patch/upstream.
> 
> With this patch from Nikhilesh, this __ atomic__ compiling issue was gone.
> The two patches fix the same issue.
> Should I abandon my patch?

The root cause is analyzed, the -latomic needs only when atomic
operations on variable are not aligned. The fix added to align variable where it was
not correct.This fix the performance issue as well, adding -latomic
will simply hide the problem by not letting to know the
alignment/performance issue. IMO, If there is no dependency with -latomic
in current code base then we should NOT add new dependency.


> 
> I see this note on: https://clang.llvm.org/docs/Toolchain.html
> Note
> Clang does not currently automatically link against libatomic when using libgcc_s. You may need to manually add -latomic to support this configuration when using non-native atomic operations (if you see link errors referring to __atomic_* functions).
> 
> commit 55fbc92d7800100628579643c9ee2770614fef10
> Author: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> Date:   Wed May 9 02:56:00 2018 +0530
> 
>     event/octeontx: fix build with clang 6
> 
>     Clang 6 & 7 fail to naturally align packed structs due to this clang
>     can't use 8byte atomic primitives and splits them into lesser atomic
>     primitives. To use lesser atomic primitives we need to link libatomic
>     (-latomic), instead supply alignment attribute to the compiler.
> 
>     timvf_worker.c:(.text+0x498): undefined reference to `__atomic_fetch_add_8'
>     timvf_worker.c:(.text+0x525): undefined reference to `__atomic_store_2'
>     timvf_worker.c:(.text+0x557): undefined reference to `__atomic_fetch_add_4'
>     timvf_worker.c:(.text+0x5de): undefined reference to `__atomic_store_2'
> 
>     Fixes: f874c1eb1519 ("event/octeontx: create and free timer
> adapter")
> 
>     Reported-by: Andrew Rybchenko <arybchenko@solarflare.com>
>     Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
>     Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>