DPDK patches and discussions
 help / color / mirror / Atom feed
From: Matan Azrad <matan@mellanox.com>
To: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>,
	"jerin.jacob@caviumnetworks.com" <jerin.jacob@caviumnetworks.com>,
	"keith.wiles@intel.com" <keith.wiles@intel.com>,
	Thomas Monjalon <thomas@monjalon.net>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v3 1/2] eal: add API to align integer to previous power of 2
Date: Wed, 4 Apr 2018 19:41:44 +0000	[thread overview]
Message-ID: <AM4PR0501MB265759D5CEACAE9E8EE69F06D2A40@AM4PR0501MB2657.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <20180404183559.GA2381@ltp-pvn>

Hi Pavan

From: Pavan Nikhilesh, Wednesday, April 4, 2018 9:36 PM
> On Wed, Apr 04, 2018 at 06:23:19PM +0000, Matan Azrad wrote:
> > Hi Pavan
> >
> > From: Pavan Nikhilesh, Wednesday, April 4, 2018 9:16 PM
> > > Hi Matan,
> > >
> > > >
> > > > Got you.
> > > > Looks like you found issue here...
> > > > The experimental tag probably should be in a root .h file.
> > > > Probably, need a fix patch to move it for a different\new .h file.
> > > >
> > > > What do you think?
> > > >
> > >
> > > Actually thats just start of the rabbit hole, if we succeed to tag a
> > > inline function in rte_common.h as experimental every lib/driver
> > > that uses rte_common.h (almost everything) needs to have CFLAGS set
> > > to - DALLOW_EXPERIMENTAL_API.
> > >
> >
> > Isn't it relevant only for the libs which are using the new tagged APIs?
> 
> Static inline functions in .h files will be added to each and every .c example
> preprocessor output for rte_pci.c which includes rte_common.h:
> 
> # 231 "/home/pavan/Work/clean/dpdk/build/include/rte_common.h"
> extern int RTE_BUILD_BUG_ON_detected_error; # 249
> "/home/pavan/Work/clean/dpdk/build/include/rte_common.h"
> static inline uint32_t __attribute__((deprecated("Symbol is not yet part of
> stable ABI"), section(".text.experimental"))) rte_combine32ms1b(register
> uint32_t x) {  x |= x >> 1;  x |= x >> 2;  x |= x >> 4;  x |= x >> 8;  x |= x >> 16;
> 
>  return x;
> }
> # 271 "/home/pavan/Work/clean/dpdk/build/include/rte_common.h"
> static inline uint64_t
> rte_combine64ms1b(register uint64_t v)
> {
>  v |= v >> 1;
>  v |= v >> 2;
>  v |= v >> 4;
>  v |= v >> 8;
>  v |= v >> 16;
>  v |= v >> 32;
> 
>  return v;
> }
> 
> Which causes compiler to throw error as DALLOW_EXPERIMENTAL_API is not
> added to cflags.
> 

Are you sure?

I added the next code and the compilation passed:
static inline uint32_t
__attribute__((deprecated("Symbol is not yet part of stable ABI"), \
section(".text.experimental")))
rte_combine32ms1b(register uint32_t x)
{
	x |= x >> 1;
	x |= x >> 2;
	x |= x >> 4;
	x |= x >> 8;
	x |= x >> 16;

	return x;
}

Actually, the combine functions should not be experimental (already used in the existed code).
It also will prevent us to add the cflag in every lib which uses the old align functions. 
Only the new align functions should be tagged.
And then, you need to add the cflag only in the places which use these functions.

Am I missing something?

> >
> > > Regards,
> > > Pavan.

  reply	other threads:[~2018-04-04 19:41 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-17 10:49 [dpdk-dev] [PATCH " Pavan Nikhilesh
2018-02-17 10:49 ` [dpdk-dev] [PATCH 2/2] test: update common auto test Pavan Nikhilesh
2018-02-18  6:11 ` [dpdk-dev] [PATCH 1/2] eal: add API to align integer to previous power of 2 Matan Azrad
2018-02-18 15:39   ` Wiles, Keith
2018-02-19  6:03     ` Matan Azrad
2018-02-19 13:55       ` Wiles, Keith
2018-02-19 14:13         ` Matan Azrad
2018-02-19 14:21           ` Wiles, Keith
2018-02-19 16:18           ` Thomas Monjalon
2018-02-19  8:36   ` Pavan Nikhilesh
2018-02-19 11:36 ` [dpdk-dev] [PATCH v2 " Pavan Nikhilesh
2018-02-19 11:36   ` [dpdk-dev] [PATCH v2 2/2] test: update common auto test Pavan Nikhilesh
2018-02-19 12:09   ` [dpdk-dev] [PATCH v2 1/2] eal: add API to align integer to previous power of 2 Matan Azrad
2018-02-26 19:10     ` Pavan Nikhilesh
2018-02-27 19:30       ` Matan Azrad
2018-04-04 10:16 ` [dpdk-dev] [PATCH v3 " Pavan Nikhilesh
2018-04-04 10:16   ` [dpdk-dev] [PATCH v3 2/2] test: update common auto test Pavan Nikhilesh
2018-04-04 12:49     ` Thomas Monjalon
2018-04-04 12:54       ` Pavan Nikhilesh
2018-04-04 16:10   ` [dpdk-dev] [PATCH v3 1/2] eal: add API to align integer to previous power of 2 Matan Azrad
2018-04-04 16:42     ` Pavan Nikhilesh
2018-04-04 17:11       ` Matan Azrad
2018-04-04 17:51         ` Pavan Nikhilesh
2018-04-04 18:10           ` Matan Azrad
2018-04-04 18:15             ` Pavan Nikhilesh
2018-04-04 18:23               ` Matan Azrad
2018-04-04 18:36                 ` Pavan Nikhilesh
2018-04-04 19:41                   ` Matan Azrad [this message]
2018-04-04 13:20 ` [dpdk-dev] [PATCH v4] " Pavan Nikhilesh
2018-04-04 15:23   ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM4PR0501MB265759D5CEACAE9E8EE69F06D2A40@AM4PR0501MB2657.eurprd05.prod.outlook.com \
    --to=matan@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=jerin.jacob@caviumnetworks.com \
    --cc=keith.wiles@intel.com \
    --cc=pbhagavatula@caviumnetworks.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).