On Wed, Apr 05, 2023 at 02:35:47PM +0200, Morten Brørup wrote: > > From: Konstantin Ananyev [mailto:konstantin.anan...@huawei.com] > > Sent: Wednesday, 5 April 2023 12.57 > > > > > > >>Another ore generic comment - do we really need to pollute all that > > > > >>code > > with RTE_TOOLCHAIN_MSVC ifdefs? > > > > >>Right now we have ability to have subdir per arch (x86/arm/etc.). > > > > >>Can we treat x86+windows+msvc as a special arch? > > > > > > > > > >i asked this question previously and confirmed in the technical board > > > > >meeting. the answer i received was that the community did not want new > > > > >directory/headers introduced for compiler support matrix and i should > > > > >use #ifdef in the existing headers. > > > > > > > > Ok, can I then ask at least to minimize number of ifdefs to absolute > > > > minimum? > > > > > > in principal no objection at all, one question though is what to do with > > > comment based documentation attached to macros? e.g. > > > > > > #ifdef SOME_FOO > > > /* some documentation */ > > > #define some_macro > > > #else > > > #define some_macro > > > #endif > > > > > > #ifdef SOME_FOO > > > /* some documentation 2 */ > > > #define some_macro2 > > > #else > > > #define some_macro2 > > > #endif > > > > > > i can either duplicate the documentation for every define so it stays > > > "attached" or i can only document the first expansion. let me know what > > > you expect. > > > > > > so something like this? > > > > > > #ifdef SOME_FOO > > > /* some documentation */ > > > #define some_macro > > > /* some documentation 2 */ > > > #define some_macro2 > > > #else > > > #define some_macro > > > #define some_macro2 > > > #endif > > > > > > or should all documentation be duplicated? which can become a teadious > > > redundancy for anyone maintaining it. keep in mind we might have to make > > > an exception for rte_common.h because it seems doing this would be > > > really ugly there. take a look let me know. > > > > My personal preference would be to keep one documentation block for both > > cases > > (yes, I suppose it needs to be updated if required): > > > > /* some documentation, probably for both SOME_FOO on/off */ > > #ifdef SOME_FOO > > #define some_macro > > #else > > #define some_macro > > #endif > > > > Or the third option of using a dummy for documentation purposes only. > rte_memcpy() does this [1], although it is an inline function and not a macro. > > [1]: > https://elixir.bootlin.com/dpdk/v23.03/source/lib/eal/include/generic/rte_memcpy.h#L90 > > No preferences here, just mentioning it! > > > > > > > > > > It is really hard to read an follow acode that is heavily ifdefed. > > Yes, and sometimes it is even harder reading code that is spread across > multiple arch-depending files. > > It is a choice between the plague or cholera. > > But it is one of the unavoidable downsides to supporting many architectures > and compilers, so we have to seek out the best compromises.
yes, there is a conditional that has to be *somewhere*. i would propose for now that it be done per-macro or whatever. but when i get the bulk of the changes in i can commit to refactoring some groups out into their own header. rte_common.h is a good candidate for this because of how many conditionals it will contain. generic/rte_common.h -> msvc/rte_common.h #include "generic/rte_common.h" -> gnu?/rte_common.h #include "generic/rte_common.h" this could carry inline/macro documentation in generic/rte_common.h but again i'd prefer to do this after msvc work is stood up at least to the point of having unit tests working before i start moving code around. for other conditionals i don't think it's worth having a whole file e.g. x86/include/rte_pause.h or something only a couple of blocks are conditional. ty
