> RTE_PTR_ADD and RTE_PTR_SUB APIs have a few limitations: > 1. ptr cast to uintptr_t drops pointer provenance and > prevents compiler optimizations > 2. return cast discards qualifiers (const, volatile) > which may hide correctness/concurrency issues. > 3. Accepts both "pointers" and "integers as pointers" which > overloads the use case and constrains the implementation > to address other challenges. > > This patch splits the API on two dimensions: > 1. pointer types
Again, thank you for all the work you put into this, Scott! > 2. integer types that represent pointers Please get rid of all the RTE_INT_PTR macros. IMO, these macros look too much like plain wrappers around simple +/- operators. It seems they are only needed for the cnxk drivers; those drivers can probably be fixed in some other way. > > This split allows addressing each of the challenges above > and provides distinct APIs for the distinct use cases. > Examples: > 1. Clang is able to optimize and improve __rte_raw_cksum > (which uses RTE_PTR_ADD) by ~40% (100 bytes) to ~8x (1.5k bytes) > TSC cycles/byte. > 2. Refactoring discovered cases that dropped qualifiers (volatile) > that the new API exposes. > > Signed-off-by: Scott Mitchell <[email protected]> > --- A few minor details follow. > static inline struct rte_mbuf * > rte_mbuf_from_indirect(struct rte_mbuf *mi) > { > + RTE_ASSERT(mi); Preferred: RTE_ASSERT(mi != NULL); > return (struct rte_mbuf *)RTE_PTR_SUB(mi->buf_addr, sizeof(*mi) + > mi->priv_size); > } > > @@ -289,6 +290,7 @@ rte_mbuf_to_baddr(struct rte_mbuf *md) > static inline void * > rte_mbuf_to_priv(struct rte_mbuf *m) > { > + RTE_ASSERT(m); Preferred: RTE_ASSERT(m != NULL); > return RTE_PTR_ADD(m, sizeof(struct rte_mbuf)); > } > > --- a/lib/mempool/rte_mempool.h > +++ b/lib/mempool/rte_mempool.h > @@ -376,6 +376,7 @@ struct __rte_cache_aligned rte_mempool { > static inline struct rte_mempool_objhdr * > rte_mempool_get_header(void *obj) > { > + RTE_ASSERT(obj); Preferred: RTE_ASSERT(obj != NULL); > return (struct rte_mempool_objhdr *)RTE_PTR_SUB(obj, > sizeof(struct rte_mempool_objhdr)); > } > @@ -399,6 +400,7 @@ static inline struct rte_mempool > *rte_mempool_from_obj(void *obj) > static inline struct rte_mempool_objtlr *rte_mempool_get_trailer(void > *obj) > { > struct rte_mempool *mp = rte_mempool_from_obj(obj); > + RTE_ASSERT(mp); Preferred: RTE_ASSERT(mp != NULL); > return (struct rte_mempool_objtlr *)RTE_PTR_ADD(obj, mp- > >elt_size); > } > > @@ -1844,6 +1846,7 @@ rte_mempool_empty(const struct rte_mempool *mp) > static inline rte_iova_t > rte_mempool_virt2iova(const void *elt) > { > + RTE_ASSERT(elt); > const struct rte_mempool_objhdr *hdr; Preferred: RTE_ASSERT(elt != NULL); Also, it might be preferable adding the RTE_ASSERT() here instead of above. > hdr = (const struct rte_mempool_objhdr *)RTE_PTR_SUB(elt, > sizeof(*hdr));
