| Message ID | 1463512877-1218-3-git-send-email-ola.liljedahl@linaro.org |
|---|---|
| State | Accepted |
| Commit | 9604e89da36d59a567c9c508f2c120ed4c6b4b5c |
| Headers | show |
odp-check passed but looks like clang is not tested there, now I have errors: CCLD odp_crypto ../../lib/.libs/libodp-linux.a(odp_timer.o): In function `_odp_atomic_u128_xchg_mm': /opt/Linaro/odp3.git/platform/linux-generic/./include/odp_atomic_internal.h:619: undefined reference to `__atomic_exchange' ../../lib/.libs/libodp-linux.a(odp_timer.o): In function `_odp_atomic_u128_cmp_xchg_mm': /opt/Linaro/odp3.git/platform/linux-generic/./include/odp_atomic_internal.h:643: undefined reference to `__atomic_compare_exchange' clang: error: linker command failed with exit code 1 (use -v to see invocation) Does somebody else see this? Maxim. On 05/17/16 22:21, Ola Liljedahl wrote: > Fixes https://bugs.linaro.org/show_bug.cgi?id=2211 > Ensure tick_buf_t structure is 128 bits large on all platforms, > regardless of support for 64-bit atomic operations. > Only assert that tick_buf_t is 128 bits large when performing > atomic operations on it (requires ODP and platform support for 128 > bit atomics). > > Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > --- > platform/linux-generic/odp_timer.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c > index 41e7195..a6d3332 100644 > --- a/platform/linux-generic/odp_timer.c > +++ b/platform/linux-generic/odp_timer.c > @@ -94,7 +94,15 @@ static odp_timeout_hdr_t *timeout_hdr(odp_timeout_t tmo) > *****************************************************************************/ > > typedef struct tick_buf_s { > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + /* No atomics support for 64-bit variables, will use separate lock */ > + /* Use the same layout as odp_atomic_u64_t but without lock variable */ > + struct { > + uint64_t v; > + } exp_tck;/* Expiration tick or TMO_xxx */ > +#else > odp_atomic_u64_t exp_tck;/* Expiration tick or TMO_xxx */ > +#endif > odp_buffer_t tmo_buf;/* ODP_BUFFER_INVALID if timer not active */ > #ifdef TB_NEEDS_PAD > uint32_t pad;/* Need to be able to access padding for successful CAS */ > @@ -105,7 +113,10 @@ ODP_ALIGNED(16) /* 16-byte atomic operations need properly aligned addresses */ > #endif > ; > > +#if __GCC_ATOMIC_LLONG_LOCK_FREE >= 2 > +/* Only assert this when we perform atomic operations on tick_buf_t */ > ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) == 16"); > +#endif > > typedef struct odp_timer_s { > void *user_ptr; > @@ -123,7 +134,11 @@ static void timer_init(odp_timer *tim, > /* All pad fields need a defined and constant value */ > TB_SET_PAD(*tb); > /* Release the timer by setting timer state to inactive */ > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + tb->exp_tck.v = TMO_INACTIVE; > +#else > _odp_atomic_u64_store_mm(&tb->exp_tck, TMO_INACTIVE, _ODP_MEMMODEL_RLS); > +#endif > } > > /* Teardown when timer is freed */ > @@ -253,7 +268,11 @@ static odp_timer_pool *odp_timer_pool_new( > tp->timers[i].queue = ODP_QUEUE_INVALID; > set_next_free(&tp->timers[i], i + 1); > tp->timers[i].user_ptr = NULL; > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + tp->tick_buf[i].exp_tck.v = TMO_UNUSED; > +#else > odp_atomic_init_u64(&tp->tick_buf[i].exp_tck, TMO_UNUSED); > +#endif > tp->tick_buf[i].tmo_buf = ODP_BUFFER_INVALID; > } > tp->tp_idx = tp_idx; > @@ -935,7 +954,11 @@ int odp_timeout_fresh(odp_timeout_t tmo) > odp_timer_pool *tp = handle_to_tp(hdl); > uint32_t idx = handle_to_idx(hdl, tp); > tick_buf_t *tb = &tp->tick_buf[idx]; > +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > + uint64_t exp_tck = tb->exp_tck.v; > +#else > uint64_t exp_tck = odp_atomic_load_u64(&tb->exp_tck); > +#endif > /* Return true if the timer still has the same expiration tick > * (ignoring the inactive/expired bit) as the timeout */ > return hdr->expiration == (exp_tck & ~TMO_INACTIVE);
On 18 May 2016 at 16:13, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > odp-check passed but looks like clang is not tested there, now I have > errors: > Bill claimed to have tested 32- and 64-bit x86 using gcc *and* clang? If your compiler defines __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16, then it should implement __atomic_exchange and __atomic_compare_exchange on 128 bit variables directly (e.g. not calling external functions, x86-64 has an instruction for this). See http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20130930/090167.html > > CCLD odp_crypto > ../../lib/.libs/libodp-linux.a(odp_timer.o): In function > `_odp_atomic_u128_xchg_mm': > /opt/Linaro/odp3.git/platform/linux-generic/./include/odp_atomic_internal.h:619: > undefined reference to `__atomic_exchange' > ../../lib/.libs/libodp-linux.a(odp_timer.o): In function > `_odp_atomic_u128_cmp_xchg_mm': > /opt/Linaro/odp3.git/platform/linux-generic/./include/odp_atomic_internal.h:643: > undefined reference to `__atomic_compare_exchange' > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > Does somebody else see this? > > Maxim. > > > On 05/17/16 22:21, Ola Liljedahl wrote: > >> Fixes https://bugs.linaro.org/show_bug.cgi?id=2211 >> Ensure tick_buf_t structure is 128 bits large on all platforms, >> regardless of support for 64-bit atomic operations. >> Only assert that tick_buf_t is 128 bits large when performing >> atomic operations on it (requires ODP and platform support for 128 >> bit atomics). >> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> >> --- >> platform/linux-generic/odp_timer.c | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/platform/linux-generic/odp_timer.c >> b/platform/linux-generic/odp_timer.c >> index 41e7195..a6d3332 100644 >> --- a/platform/linux-generic/odp_timer.c >> +++ b/platform/linux-generic/odp_timer.c >> @@ -94,7 +94,15 @@ static odp_timeout_hdr_t *timeout_hdr(odp_timeout_t >> tmo) >> >> *****************************************************************************/ >> typedef struct tick_buf_s { >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + /* No atomics support for 64-bit variables, will use separate >> lock */ >> + /* Use the same layout as odp_atomic_u64_t but without lock >> variable */ >> + struct { >> + uint64_t v; >> + } exp_tck;/* Expiration tick or TMO_xxx */ >> +#else >> odp_atomic_u64_t exp_tck;/* Expiration tick or TMO_xxx */ >> +#endif >> odp_buffer_t tmo_buf;/* ODP_BUFFER_INVALID if timer not active */ >> #ifdef TB_NEEDS_PAD >> uint32_t pad;/* Need to be able to access padding for successful >> CAS */ >> @@ -105,7 +113,10 @@ ODP_ALIGNED(16) /* 16-byte atomic operations need >> properly aligned addresses */ >> #endif >> ; >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE >= 2 >> +/* Only assert this when we perform atomic operations on tick_buf_t */ >> ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) == 16"); >> +#endif >> typedef struct odp_timer_s { >> void *user_ptr; >> @@ -123,7 +134,11 @@ static void timer_init(odp_timer *tim, >> /* All pad fields need a defined and constant value */ >> TB_SET_PAD(*tb); >> /* Release the timer by setting timer state to inactive */ >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + tb->exp_tck.v = TMO_INACTIVE; >> +#else >> _odp_atomic_u64_store_mm(&tb->exp_tck, TMO_INACTIVE, >> _ODP_MEMMODEL_RLS); >> +#endif >> } >> /* Teardown when timer is freed */ >> @@ -253,7 +268,11 @@ static odp_timer_pool *odp_timer_pool_new( >> tp->timers[i].queue = ODP_QUEUE_INVALID; >> set_next_free(&tp->timers[i], i + 1); >> tp->timers[i].user_ptr = NULL; >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + tp->tick_buf[i].exp_tck.v = TMO_UNUSED; >> +#else >> odp_atomic_init_u64(&tp->tick_buf[i].exp_tck, TMO_UNUSED); >> +#endif >> tp->tick_buf[i].tmo_buf = ODP_BUFFER_INVALID; >> } >> tp->tp_idx = tp_idx; >> @@ -935,7 +954,11 @@ int odp_timeout_fresh(odp_timeout_t tmo) >> odp_timer_pool *tp = handle_to_tp(hdl); >> uint32_t idx = handle_to_idx(hdl, tp); >> tick_buf_t *tb = &tp->tick_buf[idx]; >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> + uint64_t exp_tck = tb->exp_tck.v; >> +#else >> uint64_t exp_tck = odp_atomic_load_u64(&tb->exp_tck); >> +#endif >> /* Return true if the timer still has the same expiration tick >> * (ignoring the inactive/expired bit) as the timeout */ >> return hdr->expiration == (exp_tck & ~TMO_INACTIVE); >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On Wed, May 18, 2016 at 2:48 PM, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > On 18 May 2016 at 16:13, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > > odp-check passed but looks like clang is not tested there, now I have > > errors: > > > Bill claimed to have tested 32- and 64-bit x86 using gcc *and* clang? > Yes I did and don't see this. Maxim: what environment are you using? I'm running Ubuntu 16.04 and clang -v shows this: bill@Ubuntu15:~/linaro/timerfix2$ clang -v clang version 3.8.0-2ubuntu3 (tags/RELEASE_380/final) Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin Found candidate GCC installation: /usr/bin/../lib/gcc/i686-linux-gnu/6.0.0 Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9 Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9.3 Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/5.3.1 Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/6.0.0 Found candidate GCC installation: /usr/lib/gcc/i686-linux-gnu/6.0.0 Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.9 Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.9.3 Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/5.3.1 Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/6.0.0 Selected GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/5.3.1 Candidate multilib: .;@m64 Candidate multilib: 32;@m32 Candidate multilib: x32;@mx32 Selected multilib: .;@m64 > > If your compiler defines __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16, then it > should implement __atomic_exchange and __atomic_compare_exchange on 128 bit > variables directly (e.g. not calling external functions, x86-64 has an > instruction for this). > > See > > http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20130930/090167.html > > > > > > CCLD odp_crypto > > ../../lib/.libs/libodp-linux.a(odp_timer.o): In function > > `_odp_atomic_u128_xchg_mm': > > > /opt/Linaro/odp3.git/platform/linux-generic/./include/odp_atomic_internal.h:619: > > undefined reference to `__atomic_exchange' > > ../../lib/.libs/libodp-linux.a(odp_timer.o): In function > > `_odp_atomic_u128_cmp_xchg_mm': > > > /opt/Linaro/odp3.git/platform/linux-generic/./include/odp_atomic_internal.h:643: > > undefined reference to `__atomic_compare_exchange' > > clang: error: linker command failed with exit code 1 (use -v to see > > invocation) > > > > Does somebody else see this? > > > > Maxim. > > > > > > On 05/17/16 22:21, Ola Liljedahl wrote: > > > >> Fixes https://bugs.linaro.org/show_bug.cgi?id=2211 > >> Ensure tick_buf_t structure is 128 bits large on all platforms, > >> regardless of support for 64-bit atomic operations. > >> Only assert that tick_buf_t is 128 bits large when performing > >> atomic operations on it (requires ODP and platform support for 128 > >> bit atomics). > >> > >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> > >> --- > >> platform/linux-generic/odp_timer.c | 23 +++++++++++++++++++++++ > >> 1 file changed, 23 insertions(+) > >> > >> diff --git a/platform/linux-generic/odp_timer.c > >> b/platform/linux-generic/odp_timer.c > >> index 41e7195..a6d3332 100644 > >> --- a/platform/linux-generic/odp_timer.c > >> +++ b/platform/linux-generic/odp_timer.c > >> @@ -94,7 +94,15 @@ static odp_timeout_hdr_t *timeout_hdr(odp_timeout_t > >> tmo) > >> > >> > *****************************************************************************/ > >> typedef struct tick_buf_s { > >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > >> + /* No atomics support for 64-bit variables, will use separate > >> lock */ > >> + /* Use the same layout as odp_atomic_u64_t but without lock > >> variable */ > >> + struct { > >> + uint64_t v; > >> + } exp_tck;/* Expiration tick or TMO_xxx */ > >> +#else > >> odp_atomic_u64_t exp_tck;/* Expiration tick or TMO_xxx */ > >> +#endif > >> odp_buffer_t tmo_buf;/* ODP_BUFFER_INVALID if timer not active > */ > >> #ifdef TB_NEEDS_PAD > >> uint32_t pad;/* Need to be able to access padding for successful > >> CAS */ > >> @@ -105,7 +113,10 @@ ODP_ALIGNED(16) /* 16-byte atomic operations need > >> properly aligned addresses */ > >> #endif > >> ; > >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE >= 2 > >> +/* Only assert this when we perform atomic operations on tick_buf_t */ > >> ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) == > 16"); > >> +#endif > >> typedef struct odp_timer_s { > >> void *user_ptr; > >> @@ -123,7 +134,11 @@ static void timer_init(odp_timer *tim, > >> /* All pad fields need a defined and constant value */ > >> TB_SET_PAD(*tb); > >> /* Release the timer by setting timer state to inactive */ > >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > >> + tb->exp_tck.v = TMO_INACTIVE; > >> +#else > >> _odp_atomic_u64_store_mm(&tb->exp_tck, TMO_INACTIVE, > >> _ODP_MEMMODEL_RLS); > >> +#endif > >> } > >> /* Teardown when timer is freed */ > >> @@ -253,7 +268,11 @@ static odp_timer_pool *odp_timer_pool_new( > >> tp->timers[i].queue = ODP_QUEUE_INVALID; > >> set_next_free(&tp->timers[i], i + 1); > >> tp->timers[i].user_ptr = NULL; > >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > >> + tp->tick_buf[i].exp_tck.v = TMO_UNUSED; > >> +#else > >> odp_atomic_init_u64(&tp->tick_buf[i].exp_tck, > TMO_UNUSED); > >> +#endif > >> tp->tick_buf[i].tmo_buf = ODP_BUFFER_INVALID; > >> } > >> tp->tp_idx = tp_idx; > >> @@ -935,7 +954,11 @@ int odp_timeout_fresh(odp_timeout_t tmo) > >> odp_timer_pool *tp = handle_to_tp(hdl); > >> uint32_t idx = handle_to_idx(hdl, tp); > >> tick_buf_t *tb = &tp->tick_buf[idx]; > >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 > >> + uint64_t exp_tck = tb->exp_tck.v; > >> +#else > >> uint64_t exp_tck = odp_atomic_load_u64(&tb->exp_tck); > >> +#endif > >> /* Return true if the timer still has the same expiration tick > >> * (ignoring the inactive/expired bit) as the timeout */ > >> return hdr->expiration == (exp_tck & ~TMO_INACTIVE); > >> > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > https://lists.linaro.org/mailman/listinfo/lng-odp > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
that discussion can be moved to other my patch which adds check for clang version. Maxim. On 18 May 2016 at 23:27, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > > On Wed, May 18, 2016 at 2:48 PM, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: > >> On 18 May 2016 at 16:13, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >> >> > odp-check passed but looks like clang is not tested there, now I have >> > errors: >> > >> Bill claimed to have tested 32- and 64-bit x86 using gcc *and* clang? >> > > Yes I did and don't see this. Maxim: what environment are you using? I'm > running Ubuntu 16.04 and clang -v shows this: > > bill@Ubuntu15:~/linaro/timerfix2$ clang -v > clang version 3.8.0-2ubuntu3 (tags/RELEASE_380/final) > Target: x86_64-pc-linux-gnu > Thread model: posix > InstalledDir: /usr/bin > Found candidate GCC installation: /usr/bin/../lib/gcc/i686-linux-gnu/6.0.0 > Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9 > Found candidate GCC installation: > /usr/bin/../lib/gcc/x86_64-linux-gnu/4.9.3 > Found candidate GCC installation: > /usr/bin/../lib/gcc/x86_64-linux-gnu/5.3.1 > Found candidate GCC installation: > /usr/bin/../lib/gcc/x86_64-linux-gnu/6.0.0 > Found candidate GCC installation: /usr/lib/gcc/i686-linux-gnu/6.0.0 > Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.9 > Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/4.9.3 > Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/5.3.1 > Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/6.0.0 > Selected GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/5.3.1 > Candidate multilib: .;@m64 > Candidate multilib: 32;@m32 > Candidate multilib: x32;@mx32 > Selected multilib: .;@m64 > > >> >> If your compiler defines __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16, then it >> should implement __atomic_exchange and __atomic_compare_exchange on 128 >> bit >> variables directly (e.g. not calling external functions, x86-64 has an >> instruction for this). >> >> See >> >> http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20130930/090167.html >> >> >> > >> > CCLD odp_crypto >> > ../../lib/.libs/libodp-linux.a(odp_timer.o): In function >> > `_odp_atomic_u128_xchg_mm': >> > >> /opt/Linaro/odp3.git/platform/linux-generic/./include/odp_atomic_internal.h:619: >> > undefined reference to `__atomic_exchange' >> > ../../lib/.libs/libodp-linux.a(odp_timer.o): In function >> > `_odp_atomic_u128_cmp_xchg_mm': >> > >> /opt/Linaro/odp3.git/platform/linux-generic/./include/odp_atomic_internal.h:643: >> > undefined reference to `__atomic_compare_exchange' >> > clang: error: linker command failed with exit code 1 (use -v to see >> > invocation) >> >> >> > Does somebody else see this? >> > >> > Maxim. >> > >> > >> > On 05/17/16 22:21, Ola Liljedahl wrote: >> > >> >> Fixes https://bugs.linaro.org/show_bug.cgi?id=2211 >> >> Ensure tick_buf_t structure is 128 bits large on all platforms, >> >> regardless of support for 64-bit atomic operations. >> >> Only assert that tick_buf_t is 128 bits large when performing >> >> atomic operations on it (requires ODP and platform support for 128 >> >> bit atomics). >> >> >> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> >> >> --- >> >> platform/linux-generic/odp_timer.c | 23 +++++++++++++++++++++++ >> >> 1 file changed, 23 insertions(+) >> >> >> >> diff --git a/platform/linux-generic/odp_timer.c >> >> b/platform/linux-generic/odp_timer.c >> >> index 41e7195..a6d3332 100644 >> >> --- a/platform/linux-generic/odp_timer.c >> >> +++ b/platform/linux-generic/odp_timer.c >> >> @@ -94,7 +94,15 @@ static odp_timeout_hdr_t *timeout_hdr(odp_timeout_t >> >> tmo) >> >> >> >> >> *****************************************************************************/ >> >> typedef struct tick_buf_s { >> >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> >> + /* No atomics support for 64-bit variables, will use separate >> >> lock */ >> >> + /* Use the same layout as odp_atomic_u64_t but without lock >> >> variable */ >> >> + struct { >> >> + uint64_t v; >> >> + } exp_tck;/* Expiration tick or TMO_xxx */ >> >> +#else >> >> odp_atomic_u64_t exp_tck;/* Expiration tick or TMO_xxx */ >> >> +#endif >> >> odp_buffer_t tmo_buf;/* ODP_BUFFER_INVALID if timer not active >> */ >> >> #ifdef TB_NEEDS_PAD >> >> uint32_t pad;/* Need to be able to access padding for >> successful >> >> CAS */ >> >> @@ -105,7 +113,10 @@ ODP_ALIGNED(16) /* 16-byte atomic operations need >> >> properly aligned addresses */ >> >> #endif >> >> ; >> >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE >= 2 >> >> +/* Only assert this when we perform atomic operations on tick_buf_t */ >> >> ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) == >> 16"); >> >> +#endif >> >> typedef struct odp_timer_s { >> >> void *user_ptr; >> >> @@ -123,7 +134,11 @@ static void timer_init(odp_timer *tim, >> >> /* All pad fields need a defined and constant value */ >> >> TB_SET_PAD(*tb); >> >> /* Release the timer by setting timer state to inactive */ >> >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> >> + tb->exp_tck.v = TMO_INACTIVE; >> >> +#else >> >> _odp_atomic_u64_store_mm(&tb->exp_tck, TMO_INACTIVE, >> >> _ODP_MEMMODEL_RLS); >> >> +#endif >> >> } >> >> /* Teardown when timer is freed */ >> >> @@ -253,7 +268,11 @@ static odp_timer_pool *odp_timer_pool_new( >> >> tp->timers[i].queue = ODP_QUEUE_INVALID; >> >> set_next_free(&tp->timers[i], i + 1); >> >> tp->timers[i].user_ptr = NULL; >> >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> >> + tp->tick_buf[i].exp_tck.v = TMO_UNUSED; >> >> +#else >> >> odp_atomic_init_u64(&tp->tick_buf[i].exp_tck, >> TMO_UNUSED); >> >> +#endif >> >> tp->tick_buf[i].tmo_buf = ODP_BUFFER_INVALID; >> >> } >> >> tp->tp_idx = tp_idx; >> >> @@ -935,7 +954,11 @@ int odp_timeout_fresh(odp_timeout_t tmo) >> >> odp_timer_pool *tp = handle_to_tp(hdl); >> >> uint32_t idx = handle_to_idx(hdl, tp); >> >> tick_buf_t *tb = &tp->tick_buf[idx]; >> >> +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 >> >> + uint64_t exp_tck = tb->exp_tck.v; >> >> +#else >> >> uint64_t exp_tck = odp_atomic_load_u64(&tb->exp_tck); >> >> +#endif >> >> /* Return true if the timer still has the same expiration tick >> >> * (ignoring the inactive/expired bit) as the timeout */ >> >> return hdr->expiration == (exp_tck & ~TMO_INACTIVE); >> >> >> > >> > _______________________________________________ >> > lng-odp mailing list >> > lng-odp@lists.linaro.org >> > https://lists.linaro.org/mailman/listinfo/lng-odp >> > >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp >> > >
diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c index 41e7195..a6d3332 100644 --- a/platform/linux-generic/odp_timer.c +++ b/platform/linux-generic/odp_timer.c @@ -94,7 +94,15 @@ static odp_timeout_hdr_t *timeout_hdr(odp_timeout_t tmo) *****************************************************************************/ typedef struct tick_buf_s { +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + /* No atomics support for 64-bit variables, will use separate lock */ + /* Use the same layout as odp_atomic_u64_t but without lock variable */ + struct { + uint64_t v; + } exp_tck;/* Expiration tick or TMO_xxx */ +#else odp_atomic_u64_t exp_tck;/* Expiration tick or TMO_xxx */ +#endif odp_buffer_t tmo_buf;/* ODP_BUFFER_INVALID if timer not active */ #ifdef TB_NEEDS_PAD uint32_t pad;/* Need to be able to access padding for successful CAS */ @@ -105,7 +113,10 @@ ODP_ALIGNED(16) /* 16-byte atomic operations need properly aligned addresses */ #endif ; +#if __GCC_ATOMIC_LLONG_LOCK_FREE >= 2 +/* Only assert this when we perform atomic operations on tick_buf_t */ ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) == 16"); +#endif typedef struct odp_timer_s { void *user_ptr; @@ -123,7 +134,11 @@ static void timer_init(odp_timer *tim, /* All pad fields need a defined and constant value */ TB_SET_PAD(*tb); /* Release the timer by setting timer state to inactive */ +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + tb->exp_tck.v = TMO_INACTIVE; +#else _odp_atomic_u64_store_mm(&tb->exp_tck, TMO_INACTIVE, _ODP_MEMMODEL_RLS); +#endif } /* Teardown when timer is freed */ @@ -253,7 +268,11 @@ static odp_timer_pool *odp_timer_pool_new( tp->timers[i].queue = ODP_QUEUE_INVALID; set_next_free(&tp->timers[i], i + 1); tp->timers[i].user_ptr = NULL; +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + tp->tick_buf[i].exp_tck.v = TMO_UNUSED; +#else odp_atomic_init_u64(&tp->tick_buf[i].exp_tck, TMO_UNUSED); +#endif tp->tick_buf[i].tmo_buf = ODP_BUFFER_INVALID; } tp->tp_idx = tp_idx; @@ -935,7 +954,11 @@ int odp_timeout_fresh(odp_timeout_t tmo) odp_timer_pool *tp = handle_to_tp(hdl); uint32_t idx = handle_to_idx(hdl, tp); tick_buf_t *tb = &tp->tick_buf[idx]; +#if __GCC_ATOMIC_LLONG_LOCK_FREE < 2 + uint64_t exp_tck = tb->exp_tck.v; +#else uint64_t exp_tck = odp_atomic_load_u64(&tb->exp_tck); +#endif /* Return true if the timer still has the same expiration tick * (ignoring the inactive/expired bit) as the timeout */ return hdr->expiration == (exp_tck & ~TMO_INACTIVE);
Fixes https://bugs.linaro.org/show_bug.cgi?id=2211 Ensure tick_buf_t structure is 128 bits large on all platforms, regardless of support for 64-bit atomic operations. Only assert that tick_buf_t is 128 bits large when performing atomic operations on it (requires ODP and platform support for 128 bit atomics). Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> --- platform/linux-generic/odp_timer.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)