On Sat, Jun 22, 2019 at 07:51:10PM +0200, David Marchand wrote: > On Sat, Jun 22, 2019 at 6:17 PM Neil Horman <nhor...@tuxdriver.com> wrote: > > > On Fri, Jun 21, 2019 at 09:58:41PM +0200, David Marchand wrote: > > > On Fri, Jun 21, 2019 at 7:41 PM Neil Horman <nhor...@tuxdriver.com> > > wrote: > > > > > > > On Fri, Jun 21, 2019 at 06:47:31PM +0200, David Marchand wrote: > > > > > On Fri, Jun 21, 2019 at 6:28 PM Neil Horman <nhor...@tuxdriver.com> > > > > wrote: > > > > > > > > > > > On Fri, Jun 21, 2019 at 02:45:45PM +0200, David Marchand wrote: > > > > > > > Ok, did a new pass on the tree.. found quite some sites where we > > have > > > > > > > issues (and other discrepancies... I started a new patchset). > > > > > > > Looked at gcc documentation [1], and to me the safer approach > > would > > > > be to > > > > > > > enforce that __rte_experimental is the first thing of a symbol > > > > > > declaration. > > > > > > > > > > > > > > Comments? > > > > > > > > > > > > > Yes, thats the only way it works, in fact I'm suprised gcc didn't > > > > throw an > > > > > > error > > > > > > about expecting an asm statement if you put it anywhere else > > > > > > > > > > > > > > > > - I tried this, but then I hit issues with inlines. > > > > > Like for example: > > > > > > > > > > static inline char * __rte_experimental > > > > > rte_mbuf_buf_addr(struct rte_mbuf *mb, struct rte_mempool *mp) > > > > > { > > > > > return (char *)mb + sizeof(*mb) + rte_pktmbuf_priv_size(mp); > > > > > } > > > > > > > > > > I did not find a way to move the __rte_experimental tag without > > getting > > > > > warnings. > > > > Right, thats the way its supposed to work on gcc/icc/clang. function > > > > attributes > > > > must be declared between the return type and the function name, > > anything > > > > else > > > > will generate compiler warnings/errors. Because __rte_experimental > > > > expands to a > > > > __attribute__(...), you have to place it there. > > > > > > > > > If I try to compile some sources which includes rte_mbuf.h but > > without > > > > > -DALLOW_EXPERIMENTAL_API, then gcc errors at including the header, > > > > > complaining that rte_mbuf_buf_addr() is deprecated, even if this > > inline > > > > is > > > > > not called. > > > > > > > > > Thats...odd. I wonder if thats an artifact of the function being > > marked as > > > > inline. The compiler is supposed to insert the warning for any > > remaining > > > > calls > > > > after dead code eliminitaion. If the function is inline, I wonder if > > the > > > > compiler conservatively inserts the warning because it got expanded > > into > > > > another > > > > function, when it can't tell if it will be entirely elimintated. Can > > you > > > > provide a code sample that demonstrates this? > > > > > > > > > > > rte_mbuf_buf_addr() is called in rte_mbuf_data_addr_default(), both of > > them > > > are unused by the includers of rte_mbuf.h. > > > > > > > > > Reproduced it like this: > > > > > > [dmarchan@dmarchan ~]$ cat deprecated.c > > > __attribute__((deprecated)) static inline void *plap(void) > > > { > > > return 0; > > > } > > > > > > __attribute__((deprecated)) static inline void *plep(void) > > > { > > > plap(); > > > return 0; > > > } > > > > > > int main(int argc, char *argv[]) > > > { > > > return 0; > > > } > > > [dmarchan@dmarchan ~]$ gcc -o deprecated -Wall deprecated.c > > > deprecated.c: In function ‘plep’: > > > deprecated.c:8:2: warning: ‘plap’ is deprecated (declared at > > > deprecated.c:1) [-Wdeprecated-declarations] > > > plap(); > > > ^ > > > > > Hmm, yes, that seems buggy to me. I wonder if you are seeing this bug in > > action: > > > > https://gcc.gnu.org/bugzilla//show_bug.cgi?id=80680 > > > It has the same flavor yes. > Currently using gcc version 4.8.5 20150623 (Red Hat 4.8.5-36) (GCC) > > > > > > > Seem like the behavior fits. It would be interesting to know if clang and > > icc > > suffer from the same issue > > > > Just tried, clang is fine. > clang version 3.4.2 (tags/RELEASE_34/dot2-final) > > > Actually, I went and protected this call to rte_mbuf_buf_addr(). > And with just this, it builds fine. > I think I am going to take this approach, just a little comment :-). > Thats probably the best workaround for this at the moment, I agree. I'll add a comment to the gcc bug and see if we can't get some movement on it
thanks Neil > > -- > David Marchand