From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 43637A0547; Thu, 24 Jun 2021 18:02:30 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A0FF840040; Thu, 24 Jun 2021 18:02:29 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id BE0534003C for ; Thu, 24 Jun 2021 18:02:28 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 098FA20B6C50; Thu, 24 Jun 2021 09:02:28 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 098FA20B6C50 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1624550548; bh=Cz2jz/aKJHy17+woKRyznQGjgIjAgu8IS16UFbpJvdo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=W7bVf7feyDcJI8pTk5qipiMBWkZDGpEe2eiSALTckObnCS6vodbGZBpcKGZUAgLOi yWyViH86IT3sHflDomOF9Qljp1nKNF7ms5RvUpjlEvtQpwzoEJrXsuXlb2uJpO+6tq yBr6JyeQQoN2bEhc+4LSv1RtWDkC2EUtFPWZ+HgM= Date: Thu, 24 Jun 2021 09:02:28 -0700 From: Tyler Retzlaff To: Thomas Monjalon Cc: dev@dpdk.org, ferruh.yigit@intel.com, dmitry.kozliuk@gmail.com, david.marchand@redhat.com Message-ID: <20210624160228.GB22718@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20210623182657.GA24634@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <3277479.bKIgzLV6lZ@thomas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3277479.bKIgzLV6lZ@thomas> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [dpdk-dev] [RFC] toolchain specific macro expansion X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Jun 24, 2021 at 08:54:49AM +0200, Thomas Monjalon wrote: > 23/06/2021 20:26, Tyler Retzlaff: > > today rte_common.h defines common macros for use by dpdk and consuming > > applications. most expansions are specific to the gcc toolchain. > > > > example > > // lib/eal/include/rte_common.h > > > > /** > > * Hint never returning function > > */ > > #define __rte_noreturn __attribute__((noreturn)) > > > > there is an anticipated need rte_common.h to expose alternate > > expansions for non-gcc toolchains in the future and would like > > comments on how to achieve this in an agreeable manner. > > > > > > option 1 add conditional compilation directly to rte_common.h > > > > example > > // lib/eal/include/rte_common.h > > > > /** > > * Hint never returning function > > */ > > #ifdef RTE_TOOLCHAIN_GCC > > #define __rte_noreturn __attribute__((noreturn)) > > #endif > > > > #ifdef RTE_TOOLCHAIN_FOO > > #define __rte_noreturn __foo_expansion_for_noreturn > > #endif > > > > represents the simplest approach technically but may be tedious to > > maintain due to the number of macros and number of conditional > > expansions per macro. > > Macros are simple so the option 1 is not that bad. our preference for option 2 is based mainly on experience in platform conditional compile where reviewers of patches ask us to reduce or attempt to hide as much visible conditional compilation. the testpmd patch is a recent example where we get asked to reorganize code to avoid frequency of conditional compile for windows execenv. either way if the community prefers option 1 we'll do it that way we look forward to seeing additional responses. > > > > option 2 add toolchain specific files (follow existing platform/os > > includes pattern) > > > > example: > > // lib/eal/gcc/rte_toolchain_common.h > > #define __rte_noreturn __attribute__((noreturn)) > > We should keep a macro in rte_common.h which triggers an explicit error i think that's relatively trivial to do. rte_common.h could after toolchain specific include do a simple test. #ifndef __rte_no_return #error no __rte_no_return defined for toolchain #endif what is harder to catch is if there is an addition of a new macro to a toolchain specific header where there is no corresponding test added to rte_common.h this is somewhat mitigated by standing up more ci automation that covers the set of toolchains that may be used and to some degree would need help from patch reviewers to catch but on balance it is no worse than what we already have.