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 5E19445ADE; Tue, 8 Oct 2024 13:33:33 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 29C87402A9; Tue, 8 Oct 2024 13:33:33 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id CE21C4026F for ; Tue, 8 Oct 2024 13:33:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1728387211; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=FchMG4gqb0aP9GU1mpFpyDgpYCQUcX9E2bX0jP/ijPw=; b=WAtj+ODAB9COCPWp43rGdAdUNQCZILsx9B8B4SXOpyvoKJuieCq5bjgZWPJpgb+6BewZ0T LFQzC8aIxk/qAtNGtrRTx3qWLjxTWYhn3mWrsp2k32PHGS/BI3xE9MMuxeEYB/UN4hpoBr QlrF6H+2P95YnIzxxwh3VjMJwBDoruc= Received: from mail-lj1-f200.google.com (mail-lj1-f200.google.com [209.85.208.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-54-ewffdVJ_MgaYVxDV0GjUYg-1; Tue, 08 Oct 2024 07:33:30 -0400 X-MC-Unique: ewffdVJ_MgaYVxDV0GjUYg-1 Received: by mail-lj1-f200.google.com with SMTP id 38308e7fff4ca-2faccaed382so35725861fa.3 for ; Tue, 08 Oct 2024 04:33:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728387208; x=1728992008; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=FchMG4gqb0aP9GU1mpFpyDgpYCQUcX9E2bX0jP/ijPw=; b=iQG1jTZkWatkLSy04yWWoqtG1pj5sIn3F0//9CTAlSjhhJMlnsLtc71zq9qipHThy1 2L5eKa4A0FZMNLXSF8m35v/XV5LwlB10xKG0azkhUu85Adh7Z3/yLqsmfrrsCNAioNpT xt/N6MjFGZQYCpsSJWRUTCQ4JkG3HTPAXHzglVJKzbQO/4gFw91UHaoNnTtS28F3TaAx nirHBzpbJCDs6vTleSs42dYjiJPQUTEAJC4xmWCdhfFYvHm0kLrH6nFd5kUHslTIZ/UF gnQVAaD4Ux8tfaxI5fYh5qhG5bOV6Y3ihyc2r/TJc8oxfsuEenPMf96z4fIHTlOfNS0S UNBA== X-Gm-Message-State: AOJu0YwtRFKRz8dHAGwYgMt9RfzhTd5jfsGpFoIE3wpSF/QhJzyqYdTV PXJ8APFE7UDzhiOng8zX2LiJV++8PolfRo/i8zBj1Cve4KpZE8Eq+xCL0JzKGzycu7ay92wzv54 0qlqaxl97wGARhR5AT0k3OL/ElsByc9AkSErhGNc3TK9SYFVxu9/YJpco0P1yw7CMjXd8JEvAYe i3IXSenXHR9IS69TM= X-Received: by 2002:a05:6512:3a96:b0:533:4689:973c with SMTP id 2adb3069b0e04-539ab876ca6mr8138804e87.23.1728387208480; Tue, 08 Oct 2024 04:33:28 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFqFnPo/AnB3lXfIcJTVscdGVm1YBPR5kfTnaX61aFf1Z4RYY09lWsA6kYe6LAc6AsqyO87h8WAbU3UqoNBUb0= X-Received: by 2002:a05:6512:3a96:b0:533:4689:973c with SMTP id 2adb3069b0e04-539ab876ca6mr8138789e87.23.1728387208013; Tue, 08 Oct 2024 04:33:28 -0700 (PDT) MIME-Version: 1.0 References: <20240930175033.2283861-1-bruce.richardson@intel.com> <20241001111802.2728765-1-bruce.richardson@intel.com> In-Reply-To: From: David Marchand Date: Tue, 8 Oct 2024 13:33:16 +0200 Message-ID: Subject: Re: [PATCH v2 0/8] centralize AVX-512 feature detection To: Bruce Richardson Cc: dev@dpdk.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 On Tue, Oct 8, 2024 at 12:03=E2=80=AFPM Bruce Richardson wrote: > > On Tue, Oct 08, 2024 at 10:49:39AM +0200, David Marchand wrote: > > On Tue, Oct 1, 2024 at 1:19=E2=80=AFPM Bruce Richardson > > wrote: > > > > > > The meson code to detect CPU and compiler support for AVX512 was dupl= icated > > > across multiple drivers. Do all detection in just a single place to s= implify > > > the code. > > > > > > v2: ensure that target_has_avx512 is always defined on x86 to fix bui= ld errors > > > > > > Bruce Richardson (8): > > > config/x86: add global defines for checking AVX-512 > > > event/dlb2: use global AVX-512 variables > > > common/idpf: use global AVX-512 variables > > > net/cpfl: use global AVX-512 variables > > > net/i40e: use global AVX-512 variables > > > net/iavf: use global AVX-512 variables > > > net/ice: use global AVX-512 variables > > > net/idpf: use global AVX-512 variables > > > > > > config/x86/meson.build | 19 +++++++++++---- > > > drivers/common/idpf/meson.build | 17 ++----------- > > > drivers/event/dlb2/meson.build | 42 +++++++------------------------= -- > > > drivers/net/cpfl/meson.build | 19 ++------------- > > > drivers/net/i40e/meson.build | 13 ++-------- > > > drivers/net/iavf/meson.build | 13 ++-------- > > > drivers/net/ice/meson.build | 15 ++---------- > > > drivers/net/idpf/meson.build | 19 ++------------- > > > 8 files changed, 36 insertions(+), 121 deletions(-) > > > > Thanks for this cleanup, I have two comments. > > > > - Some drivers were going into great lenghts to check that individiual > > avx512 features were available. > > With this series, we end up requiring support for all features to > > announce avx512 availability. > > Are we perhaps disabling AVX512 support with some toolchains, out > > there, supporting only part of the set? > > > > The various AVX-512 feature sets checked for (F, BW, VL, DQ) were all > introduced in the same hardware generation - all are available in gcc whe= n > using -march=3Dskylake-avx512 or later, or -march=3Dznver4. On the toolch= ain > side, gcc introduced all these flags simultaneously in gcc-6 [1]. For > clang/llvm, testing with godbolt for compiler errors/warnings indicates > that all these 4 avx512 flags are available from clang 3.6 - the minimum = we > support in DPDK [2] > > [1] https://gcc.gnu.org/gcc-6/changes.html > [2] https://doc.dpdk.org/guides/linux_gsg/sys_reqs.html#compilation-of-th= e-dpdk Perfect, thanks for the details. > > > - Some drivers were checking for presence of -mno-avx512f in > > machine_args as a way to disable building any AVX512 stuff. > > This gets discarded with this series. > > > > Yes, because it should no longer be necessary. The places in the build > system where we set the no-avx512f flag are reworked so that we don't hav= e > cc_has_avx512 set. Ok, it is clearer now. Last comment on style: $ git grep cc_avx512_flags drivers/ drivers/common/idpf/meson.build: avx512_args =3D [cflags] + cc_avx51= 2_flags drivers/event/dlb2/meson.build: c_args: cflags + cc_avx512_flags) drivers/net/i40e/meson.build: avx512_args =3D cflags + cc_avx512_fla= gs drivers/net/iavf/meson.build: avx512_args =3D cflags + cc_avx512_fla= gs drivers/net/ice/meson.build: avx512_args =3D cflags + cc_avx512_flag= s I think it is safe to remove the [] around cflags in common/idpf, right? Do you have some cycles to send a v2 and convert lib/net and net/virtio ? Otherwise, can you do a followup patch for rc2? Thanks. --=20 David Marchand