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 8B077A04B3; Mon, 27 Jan 2020 21:38:50 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3B0E91BFCD; Mon, 27 Jan 2020 21:38:49 +0100 (CET) Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 7E1B31BF75; Mon, 27 Jan 2020 21:38:47 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id BE99822205; Mon, 27 Jan 2020 15:38:46 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Mon, 27 Jan 2020 15:38:46 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=7FLQc7Y18TcrLfmMAt+r7/GTjUckD+9iQVhHOS52GHI=; b=asD81oe+7ErP sVfRaHKdHThuw5QHL+cosoe695s9RzNu+ku/9QCoE/eAvFP4zADclGm+uoVO0Q2l u+zQ4dL4vl4iErD+tcpSFsvqb2HZ11u2TL3HYavoFpRiWpAeUdSXf+MLDIJHuAl2 HHwBArNkHajUBZy5z67V8IjNmvXqebY= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=7FLQc7Y18TcrLfmMAt+r7/GTjUckD+9iQVhHOS52G HI=; b=jOi0mWGuI4cax9hY/TeCk/stYyl8vTSLiAWaDsNEj6DJQPtpejjoaSNnB Xl13euyJWQRWpFa/3MbZHWe7PN0utrwGu8krIQ45HBeTDmcIODkKVVUjO2xb5qpz OVTGrn52ky2lWTa0+iDScP3pYpBbfSGUh1BrPSH8s/VYfigKboLa0ZkzvqmZHT2u kA2uRiyuLvSwgwCklwmyee3lTr+ttGYTFk8nf77mkFmL8kLrtRFwXNuQzVVULjhh w5Gv3x+Mli8srnkDS5+aUviTjYAtq13BwIoZQxGUSvfYWtS4kyzIOTmuto6TKw98 ekKtN/7cE07JpQG0CecuFHWu8UpKQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedugedrfedvgddufeejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecuff homhgrihhnpehvrghrshdrmhhkpdguphgukhdrohhrghenucfkphepjeejrddufeegrddv tdefrddukeegnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 951A03280063; Mon, 27 Jan 2020 15:38:45 -0500 (EST) From: Thomas Monjalon To: Ferruh Yigit Cc: Alexander Kozyrev , dev@dpdk.org, rasland@mellanox.com, matan@mellanox.com, viacheslavo@mellanox.com, techboard@dpdk.org Date: Mon, 27 Jan 2020 21:38:43 +0100 Message-ID: <1657405.axiByQ7kbq@xps> In-Reply-To: References: <1579789555-23239-1-git-send-email-akozyrev@mellanox.com> <2508825.icZtzsIVVb@xps> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v2 1/5] mk/icc: disable treatment of warnings as errors 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" 27/01/2020 16:37, Ferruh Yigit: > On 1/24/2020 7:37 PM, Thomas Monjalon wrote: > > 24/01/2020 17:36, Ferruh Yigit: > >> On 1/23/2020 6:20 PM, Alexander Kozyrev wrote: > >>> Remove -Werror-all flag in ICC configuration file to stop treating ICC > >>> warnings as errors in DPDK due to many false positives. We are using > >>> GCC and Clang as a benchmark for warnings anyway for simplification. > >>> > >>> Suggested-by: Thomas Monjalon > >>> Signed-off-by: Alexander Kozyrev > >>> Acked-by: Viacheslav Ovsiienko > >>> Acked-by: Thomas Monjalon > >>> --- > >>> mk/toolchain/icc/rte.vars.mk | 4 ---- > >>> 1 file changed, 4 deletions(-) > >>> > >>> diff --git a/mk/toolchain/icc/rte.vars.mk b/mk/toolchain/icc/rte.vars.mk > >>> index 8aa87aa..1729f3d 100644 > >>> --- a/mk/toolchain/icc/rte.vars.mk > >>> +++ b/mk/toolchain/icc/rte.vars.mk > >>> @@ -47,10 +47,6 @@ WERROR_FLAGS += -diag-disable 13368 -diag-disable 15527 > >>> WERROR_FLAGS += -diag-disable 188 > >>> WERROR_FLAGS += -diag-disable 11074 -diag-disable 11076 -Wdeprecated > >>> > >>> -ifeq ($(RTE_DEVEL_BUILD),y) > >>> -WERROR_FLAGS += -Werror-all > >>> -endif > >> > >> Not sure about removing this globally, as of now the ICC builds fine. If this is > >> for the coming changes in mlx, why not disable warnings in mlx driver only? > > > > Adding special handling for ICC in the driver means it is kind of supported. > > ICC support is on best-effort basis as a favor to Intel and its CI. > > > > I don't see any good reason to spend time disabling ICC warnings each time > > we hit a false positive. > > And I would like we stop doing strange things in the code to make ICC happy. > > We do not need to support ICC, or at least we do not need to make ICC build > > warning-free. > > > > This patch will solve all future annoying issues with ICC in the future. > > > > Now let me ask the reverse question: why the flag -Werror-all is set for ICC? > > We set this flag for gcc and clang because we use gcc and clang as static > > code analyzers (like coverity) and we want to be sure to catch any issue. > > ICC is not used as code analyzer, this is just an optimization for some > > Intel projects. I won't elaborate on the packaging and licensing of ICC... > > > > I hope you will be convinced to acknowledge this change. > > > > To support the ICC or not is a valid question, but for me it is better to > support more compilers in case different ones catch an additional issues that is > a benefit for us, although I agree false positives are annoying, which is not > limited to icc. > > "-Werror-all" is forced in DEVEL_BUILD, in this cause I was assuming to force > developer to fix the warning to increase the code quality, independent from > compiler. I doubt that ICC can help to increase code quality. And it is really annoying to fix any ICC warning, because it is not a truly open compiler. > As of now, all DPDK compiles with icc without warning. There is no warning currently because we avoid some known patterns that ICC does not like. I don't see this fact as an advantage. > But we are allowing the > icc warnings just because the warnings with the change in the mlx driver. And > when we let is once, it is for sure warnings will increase by time. Yes, this is the intent of this patch: let the ICC warnings grow. > If we support ICC, I am for keeping this flag and support it properly. We do not really support ICC. See the first item of the requirements: http://doc.dpdk.org/guides/linux_gsg/sys_reqs.html#compilation-of-the-dpdk "supported C compiler such as gcc (version 4.9+) or clang (version 3.4+)" and the consensus in the techboard: http://mails.dpdk.org/archives/dev/2019-June/135847.html "Agreement that all DPDK PMDs themselves must be possible to compile with a DPDK supported compiler - GCC and/or clang" I add the Technical Board as Cc, in case a confirmation is required.