DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: "Tyler Retzlaff" <roretzla@linux.microsoft.com>,
	"Mattias Rönnblom" <hofors@lysator.liu.se>
Cc: dev@dpdk.org
Subject: Re: [PATCH] eal: provide rte attribute macro for GCC attribute
Date: Sun, 18 Feb 2024 16:31:50 +0100	[thread overview]
Message-ID: <3474620.eGJsNajkDb@thomas> (raw)
In-Reply-To: <3838403f-af2e-4a2c-b06a-ec4db956d410@lysator.liu.se>

18/02/2024 15:51, Mattias Rönnblom:
> On 2024-02-18 13:24, Thomas Monjalon wrote:
> > 15/02/2024 23:20, Tyler Retzlaff:
> >> Provide a new macro __rte_attribute(a) that when directly used
> >> compiles to empty for MSVC and to __attribute__(a) when using GCC/LLVM.
> >>
> >> Replace direct use of __attribute__ in __rte_xxx macros where there is
> >> existing empty expansion of the macro for MSVC allowing removal of
> >> repeated #ifdef RTE_TOOLCHAIN_MSVC per macro to expand empty.
> > 
> > I'm not sure it makes sense.
> > I prefer seeing clearly what is empty with MSVC.
> 
> Anything __rte_attribute() is empty on MSVC. You could rename it 
> __rte_attribute_ignored_by_msvc() for clarity.

Yes it would bring more clarity.
But I still prefer #ifdef which may work with more compilers.

> One could note that on the ignore list are things like "may_alias" and 
> "packed", so whatever comes out of a MSVC build should not be expected 
> to actually work.
> 
> Unless I'm missing something, for all the attributes that will have 
> MSVC-propriety equivalent, the usage pattern would have to change, since 
> the syntax is different in incompatible ways.
> 
> Wouldn't it be better to ask the MSVC team to add support GCC 
> attributes? ICC and LLVM managed, so why not Microsoft. Then you would 
> solve this issue for all Open Source projects, not only DPDK.

We can expect MSVC to improve.
That's another reason why I prefer to keep #ifdef to keep track easily.



  reply	other threads:[~2024-02-18 15:31 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-15 22:20 [PATCH] remove some MSVC conditional compile to empty Tyler Retzlaff
2024-02-15 22:20 ` [PATCH] eal: provide rte attribute macro for GCC attribute Tyler Retzlaff
2024-02-18 12:24   ` Thomas Monjalon
2024-02-18 12:53     ` Morten Brørup
2024-02-18 15:34       ` Thomas Monjalon
2024-02-18 16:38         ` Morten Brørup
2024-02-18 16:44           ` Thomas Monjalon
2024-02-20 17:50             ` Tyler Retzlaff
2024-02-18 14:51     ` Mattias Rönnblom
2024-02-18 15:31       ` Thomas Monjalon [this message]
2024-02-20 18:06         ` Tyler Retzlaff
2024-02-20 18:27           ` Thomas Monjalon
2024-02-27 22:45 ` [PATCH] remove some MSVC conditional compile to empty Tyler Retzlaff

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=3474620.eGJsNajkDb@thomas \
    --to=thomas@monjalon.net \
    --cc=dev@dpdk.org \
    --cc=hofors@lysator.liu.se \
    --cc=roretzla@linux.microsoft.com \
    /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).