> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Joyce Kong
> Sent: Thursday, April 16, 2020 7:39 AM
> 
> Bitwise operation APIs are defined and used in a lot of PMDs,
> which caused a huge code duplication. To reduce duplication,
> this patch consolidates them into a common API family.
> 

[...]

> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> notice
> + *
> + * Return the original bit from a 32-bit value, then set it to 1 without
> + * memory ordering.
> + *
> + * @param nr
> + *   The target bit to get and set.
> + * @param addr
> + *   The address holding the bit.
> + * @return
> + *   The original bit.
> + */
> +__rte_experimental
> +static inline uint32_t
> +rte_test_and_set_bit32_relaxed(unsigned int nr, volatile uint32_t *addr)
> +{
> +     RTE_ASSERT(nr < 32);
> +
> +     uint32_t mask = UINT32_C(1) << nr;
> +     uint32_t val = *addr;
> +     *addr = (*addr) | mask;

Suggestion:
-       *addr = (*addr) | mask;
+       *addr = val | mask;

> +     return val & mask;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> notice
> + *
> + * Return the original bit from a 32-bit value, then clear it to 0 without
> + * memory ordering.
> + *
> + * @param nr
> + *   The target bit to get and clear.
> + * @param addr
> + *   The address holding the bit.
> + * @return
> + *   The original bit.
> + */
> +__rte_experimental
> +static inline uint32_t
> +rte_test_and_clear_bit32_relaxed(unsigned int nr, volatile uint32_t *addr)
> +{
> +     RTE_ASSERT(nr < 32);
> +
> +     uint32_t mask = UINT32_C(1) << nr;
> +     uint32_t val = *addr;
> +     *addr = (*addr) & (~mask);

Suggestion:
-       *addr = (*addr) & (~mask);
+       *addr = val & (~mask);

> +     return val & mask;
> +}
> +
> +/*---------------------------- 64 bit operations -------------------------
> ---*/
> +

[...]

> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> notice
> + *
> + * Return the original bit from a 64-bit value, then set it to 1 without
> + * memory ordering.
> + *
> + * @param nr
> + *   The target bit to get and set.
> + * @param addr
> + *   The address holding the bit.
> + * @return
> + *   The original bit.
> + */
> +__rte_experimental
> +static inline uint64_t
> +rte_test_and_set_bit64_relaxed(unsigned int nr, volatile uint64_t *addr)
> +{
> +     RTE_ASSERT(nr < 64);
> +
> +     uint64_t mask = UINT64_C(1) << nr;
> +     uint64_t val = *addr;
> +     *addr = (*addr) | mask;

Suggestion:
-       *addr = (*addr) | mask;
+       *addr = val | mask;

> +     return val;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> notice
> + *
> + * Return the original bit from a 64-bit value, then clear it to 0 without
> + * memory ordering.
> + *
> + * @param nr
> + *   The target bit to get and clear.
> + * @param addr
> + *   The address holding the bit.
> + * @return
> + *   The original bit.
> + */
> +__rte_experimental
> +static inline uint64_t
> +rte_test_and_clear_bit64_relaxed(unsigned int nr, volatile uint64_t *addr)
> +{
> +     RTE_ASSERT(nr < 64);
> +
> +     uint64_t mask = UINT64_C(1) << nr;
> +     uint64_t val = *addr;
> +     *addr = (*addr) & (~mask);

Suggestion:
-       *addr = (*addr) & (~mask);
+       *addr = val & (~mask);

> +     return val & mask;
> +}
> +
> +#endif /* _RTE_BITOPS_H_ */
> --
> 2.17.1
> 

Reply via email to