From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4C178A2EDB for ; Tue, 1 Oct 2019 15:23:52 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6C6B34CA7; Tue, 1 Oct 2019 15:23:51 +0200 (CEST) Received: from mail-lj1-f196.google.com (mail-lj1-f196.google.com [209.85.208.196]) by dpdk.org (Postfix) with ESMTP id C60CF37B4 for ; Tue, 1 Oct 2019 15:23:49 +0200 (CEST) Received: by mail-lj1-f196.google.com with SMTP id b20so13355875ljj.5 for ; Tue, 01 Oct 2019 06:23:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=p+LVpIPMaAmVlbsxIZHh7VdKFATxhSfkZZ7DaouQcjg=; b=RUfHESkG0n1xbVmo9MtXRFemo6bwPBipqernQzvBhQ5P4KuBIqzDZM28SH/npZU1J0 j6VPYM10UiCU1FS3euF2nkUj4Nb4Zl+Rz0Mhw8rPxGNkXdqCrCsI+2Y7aLZtN0SrJB4k T2MsvyhCO6+n47lDTLmypUXH6MnjZ/farj2/aSMe6GzR42QrG1LFvk4mqBNWZfbcodWP 9xfwPrdHcLdeZklQAnmIzlbwRiHA63mkKaJUqZ9tCw5fC8W4EyTKQZqPSrhvfyNAaIqo DMcuaoj2eGaS92qlz/QCjdsIw8tQsj8TiGibWB7UX7St+lO1gNIWxLOphRbYeBz3voia FsMA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=p+LVpIPMaAmVlbsxIZHh7VdKFATxhSfkZZ7DaouQcjg=; b=UIuB6xhZbEdD3KcQ1QExlH+cNaaz5HI8ZLL9rjIBt90n85mZU0N8WVLFmkPHVVpWRe FDwd/TwdPZK4lTQFH64Xp+DjiiizE4Avhpka/RF/gzbnIp9C52Dn2nt118FbS9cLKN37 fnhbkSKslvI4BWeBIVr+svaFI+ys+43qo0ASOuw16I9jh5WPBceAHXpQ6ajp/VQQgXx0 NFZcv6DHjdJ622/ZTPYPHxV34BnY7JeTlGHt4ND3qaQ8Dd8Bq0ocHPqKQ+tA/JNX3CGl BWVMz5qrJAX9pBJCYCBVf4uM3ks6VBnBPYihhNqYl8kS5S5DWkFKlPZOoSosjNxLlSOt Igog== X-Gm-Message-State: APjAAAX7i+pJYcqKtIDJYCRjA4ODJgC606686Oni+oYykyNHaB691oXi 0VkWLYNQoPfQ8auEI1LPjeUvQw== X-Google-Smtp-Source: APXvYqwe3zH++88UB0vz0EuFBHazZ0mMo/1Grq/uPWsnFbd1QHIeK3ruxFlUfDYuK53ZHPN4AUN59A== X-Received: by 2002:a2e:87cb:: with SMTP id v11mr101707ljj.31.1569936229213; Tue, 01 Oct 2019 06:23:49 -0700 (PDT) Received: from [10.0.0.72] (31-172-191-173.noc.fibertech.net.pl. [31.172.191.173]) by smtp.gmail.com with ESMTPSA id n25sm4038607ljc.107.2019.10.01.06.23.48 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 01 Oct 2019 06:23:48 -0700 (PDT) To: Bruce Richardson , dev@dpdk.org Cc: Thomas Monjalon , Ray Kinsella , Neil Horman , bluca@debian.org References: <20190927194932.22197-1-bruce.richardson@intel.com> <20190927205931.23022-1-bruce.richardson@intel.com> <20190927205931.23022-3-bruce.richardson@intel.com> From: Andrzej Ostruszka Message-ID: Date: Tue, 1 Oct 2019 15:23:47 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190927205931.23022-3-bruce.richardson@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 2/2] build: support building ABI versioned files twice X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" Thanks Bruce for the patch. I like the idea of splitting versioning out of rte_compat.h, but I have some comments. On 9/27/19 10:59 PM, Bruce Richardson wrote: [...] > --- a/config/common_base > +++ b/config/common_base > @@ -111,6 +111,7 @@ CONFIG_RTE_MAX_VFIO_CONTAINERS=64 > CONFIG_RTE_MALLOC_DEBUG=n > CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n > CONFIG_RTE_USE_LIBBSD=n > +CONFIG_RTE_USE_FUNCTION_VERSIONING=y I'm not fond of this config option - it is not really an option to be changed by the user. I would prefer to just add flag to CFLAGS in mk/target/generic/rte.vars.mk. > # > # Recognize/ignore the AVX/AVX512 CPU flags for performance/power testing. > diff --git a/config/rte_config.h b/config/rte_config.h > index 0bbbe274f..b63a2fdea 100644 > --- a/config/rte_config.h > +++ b/config/rte_config.h > @@ -31,9 +31,6 @@ > > /****** library defines ********/ > > -/* compat defines */ > -#define RTE_BUILD_SHARED_LIB > - So now everything builds "as static lib" (but with "-fPIC") apart from those libraries that use symbol versioning. I'm OK with that however I'd like to note that code might be using RTE_BUILD_SHARED_LIB and do different things e.g. app/test-bbdev/test_bbdev_perf.c. I know that was already the case - just wanted to say that aloud to make sure we are all aware of this :). > diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst > index 449b33494..e95a1a2be 100644 > --- a/doc/guides/contributing/coding_style.rst > +++ b/doc/guides/contributing/coding_style.rst > @@ -948,6 +948,13 @@ reason > built. For missing dependencies this should be of the form > ``'missing dependency, "libname"'``. > > +use_function_versioning > + **Default Value = false**. > + Specifies if the library in question has ABI versioned functions. If it > + has, this value should be set to ensure that the C files are compiled > + twice with suitable parameters for each of shared or static library > + builds. > + Maybe a different name for this option? In general an "ideal theoretical" solution would be for build system to figure out on its own that separate build is necessary automatically - but that might incur some performance penalty (additional grep'ing of sources or so). So I'm fine with this option however I'd like to rename it to actually indicate what it's effect is. Like 'separate_build' or 'split_build' or 'rebuild_objects' or ... The intention of using of versioned symbols is already indicated by inclusion of the relevant header. > diff --git a/lib/librte_eal/common/include/rte_function_versioning.h b/lib/librte_eal/common/include/rte_function_versioning.h > index ce963d4b1..55e88ffae 100644 > --- a/lib/librte_eal/common/include/rte_function_versioning.h > +++ b/lib/librte_eal/common/include/rte_function_versioning.h > @@ -7,6 +7,10 @@ > #define _RTE_FUNCTION_VERSIONING_H_ > #include > > +#ifndef RTE_USE_FUNCTION_VERSIONING > +#error Use of function versioning disabled, is "use_function_versioning=true" in meson.build? > +#endif If you accept above suggestion then change this message to something like: "Function versioning requires 'separate_build=true' in meson.build" BTW it turned out that this need for separate build for versioned symbols is a result of long standing gcc bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48200 I'll test this with clang and if this will work then maybe we could guard this #if with another check for 'gcc'. Best regards Andrzej Tested-by: Andrzej Ostruszka