On Fri, Jan 03, 2025 at 01:26:34PM -0800, Andre Muezerie wrote: > On Fri, Jan 03, 2025 at 11:24:02AM -0800, Stephen Hemminger wrote: > > On Fri, 3 Jan 2025 07:36:48 -0800 > > Andre Muezerie <andre...@linux.microsoft.com> wrote: > > > > > From: Andre Muezerie <andre...@linux.microsoft.com> > > > To: andre...@linux.microsoft.com > > > Cc: dev@dpdk.org, step...@networkplumber.org > > > Subject: [PATCH v11 0/3] add diagnostics macros to make code portable > > > Date: Fri, 3 Jan 2025 07:36:48 -0800 > > > X-Mailer: git-send-email 1.8.3.1 > > > > > > It was a common pattern to have "GCC diagnostic ignored" pragmas > > > sprinkled over the code and only activate these pragmas for certain > > > compilers (gcc and clang). Clang supports GCC's pragma for > > > compatibility with existing source code, so #pragma GCC diagnostic > > > and #pragma clang diagnostic are synonyms for Clang > > > (https://clang.llvm.org/docs/UsersManual.html). > > > > > > Now that effort is being made to make the code compatible with MSVC > > > these expressions would become more complex. It makes sense to hide > > > this complexity behind macros. This makes maintenance easier as these > > > macros are defined in a single place. As a plus the code becomes > > > more readable as well. > > > > Since 90% of these cases are about removing const from a pointer, > > maybe it would be better to have a macro that did that? > > > > Would not work for base driver code which is pretending to be platform > > independent. > > Most of the warnings I've seen were about dropping the volatile qualifier, > like the one below: > > ../drivers/net/i40e/i40e_rxtx_vec_sse.c:42:32: warning: cast from 'volatile > struct i40e_32byte_rx_desc::(unnamed at > ../drivers/net/i40e/base/i40e_type.h:803:2) *' to > '__attribute__((__vector_size__(2 * sizeof(long long)))) long long *' drops > volatile qualifier [-Wcast-qual] > 42 | _mm_store_si128((__m128i > *)&rxdp[i].read, > | ^ > > To make sure I understood your suggestion correctly, you're proposing to > replace this > > __rte_diagnostic_push > __rte_diagnostic_ignored_wcast_qual > _mm_store_si128((__m128i *)&rxdp[i].read, dma_addr0); > __rte_diagnostic_pop > > > with something like this? > > _mm_store_si128(RTE_IGNORE_CAST_QUAL((__m128i *)&rxdp[i].read), > dma_addr0); > > This could be done, and I think it does look better, despite the slight line > length increase.
+1 for this option. One macro can be used to drop all qualifiers, both const and volatile, right?