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 CF3D2A04BC; Thu, 8 Oct 2020 15:03:17 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AE93F1BFCF; Thu, 8 Oct 2020 15:03:15 +0200 (CEST) Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by dpdk.org (Postfix) with ESMTP id 55F6D1BFCC for ; Thu, 8 Oct 2020 15:03:13 +0200 (CEST) Received: by mail-wm1-f68.google.com with SMTP id q5so6439191wmq.0 for ; Thu, 08 Oct 2020 06:03:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=VpoEmbYenNQIK6dE+JpGVILNmWjMIZc2X0jiqcho9DE=; b=RWyuScB1ECPVJ+4rHVr3434SMTOtBldz2qRHz3IqOEQcRvvdoH9/hEBilnFsaT2W/T T66GfAf9seti2JfSHR18sX2SWPrzt9aHC4vFTa5ZCsgzVg6TFxNfQK6WN2TNo4Umcef6 t/xm+Hj95gL6wdURNmsSHcUed02S9qXyJyeJiGOH7JcMAMh4ZxEMjViisG8NHbqLoukZ /3oHEelQPpuY3Joj95eOoknaMQBR0wl6XKv9ikWit7ykxzxs5R/gO00YWEJxVQncDH8K 7pcUDmYoONjoHkg08GHtJEESE/t5n5zVzxBplGQiF8RDq9h+d7PNLUGSOLqv4g58Jek5 2Y1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=VpoEmbYenNQIK6dE+JpGVILNmWjMIZc2X0jiqcho9DE=; b=m220WKHTtbgOxh+7o0KByVmSJ7RJWcyP/kNrlNyThT0hepYyZkOiggSTIRSC2bjR9P Eg/72h8MDOcWa0VqER216VNYsDJ9xnt2YjGef37R/x+hSkEAdMg7UkhNXTLD90VcsQdH N3qkgn8PhVmvT1Ek1gS1cnJfpUJIM7FK8mvZRNdmC1EjY1mX0qWA/oXrqyO/YguJ8h0F rkIKCAX0fbl9soTIg0iKn205dq99gHsBQuNB3Kl2Yi8t25LA2CJ1Sa4ehcVSeNWcBpre F19adGjQxJhZN6EqTa7PPe8i8lU8Syqygm05P+Sof4U4VuxFSZSrZway2QqqfctmcIoI rsgg== X-Gm-Message-State: AOAM533FGyNOSa01e/SnwykR2lOkLrATJW3Adyu4w7fo0igikOnNiyQT U3mrCUsx1drU3ACK6YOsDusCcw== X-Google-Smtp-Source: ABdhPJxsXoaVLc7KL1Pt9LS9UGp79WdowTKE1GsU00j0uJ00yGiCcLshjjw7Y07ZiaqDdy50EvR/5A== X-Received: by 2002:a05:600c:22d2:: with SMTP id 18mr3304551wmg.66.1602162187974; Thu, 08 Oct 2020 06:03:07 -0700 (PDT) Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:3456:36f7:e65e:d1a0]) by smtp.gmail.com with ESMTPSA id m3sm8043198wme.3.2020.10.08.06.03.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Oct 2020 06:03:06 -0700 (PDT) Date: Thu, 8 Oct 2020 15:03:06 +0200 From: Olivier Matz To: Bruce Richardson Cc: "Power, Ciara" , "dev@dpdk.org" , Ray Kinsella , Neil Horman Message-ID: <20201008130306.GX21395@platinum> References: <20200807155859.63888-1-ciara.power@intel.com> <20200930130415.11211-1-ciara.power@intel.com> <20200930130415.11211-2-ciara.power@intel.com> <20201006093217.GG21395@platinum> <20201007111812.GR21395@platinum> <20201008100401.GV21395@platinum> <20201008114847.GC1106@bricha3-MOBL.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201008114847.GC1106@bricha3-MOBL.ger.corp.intel.com> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth 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" On Thu, Oct 08, 2020 at 12:48:47PM +0100, Bruce Richardson wrote: > On Thu, Oct 08, 2020 at 11:58:08AM +0100, Power, Ciara wrote: > > Hi Olivier, > > > > > > >-----Original Message----- From: Olivier Matz > > >Sent: Thursday 8 October 2020 11:04 To: Power, Ciara > > > Cc: dev@dpdk.org; Ray Kinsella ; > > >Neil Horman ; Richardson, Bruce > > > Subject: Re: [dpdk-dev] [PATCH v3 01/18] > > >eal: add max SIMD bitwidth > > > > > >Hi Ciara, > > > > > >On Thu, Oct 08, 2020 at 09:25:42AM +0000, Power, Ciara wrote: > > >> Hi Olivier, > > >> > > >> > > >> >-----Original Message----- From: Olivier Matz > > >> > Sent: Wednesday 7 October 2020 12:18 To: > > >> >Power, Ciara Cc: dev@dpdk.org; Ray Kinsella > > >> >; Neil Horman ; Richardson, > > >> >Bruce Subject: Re: [dpdk-dev] [PATCH v3 > > >> >01/18] eal: add max SIMD bitwidth > > >> > > > >> >Hi Ciara, > > >> > > > >> >On Wed, Oct 07, 2020 at 10:47:34AM +0000, Power, Ciara wrote: > > >> >> Hi Olivier, > > >> >> > > >> >> Thanks for reviewing, some comments below. > > >> >> > > >> >> > > >> >> >-----Original Message----- From: Olivier Matz > > >> >> > Sent: Tuesday 6 October 2020 10:32 To: > > >> >> >Power, Ciara Cc: dev@dpdk.org; Ray > > >> >> >Kinsella ; Neil Horman > > >> >> >Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD > > >> >> >bitwidth > > >> >> > > > >> >> >Hi Ciara, > > >> >> > > > >> >> >Please find some comments below. > > >> >> > > > >> >> >On Wed, Sep 30, 2020 at 02:03:57PM +0100, Ciara Power wrote: > > >> >> >> This patch adds a max SIMD bitwidth EAL configuration. The API > > >> >> >> allows for an app to set this value. It can also be set using > > >> >> >> EAL argument --force-max-simd-bitwidth, which will lock the > > >> >> >> value and override any modifications made by the app. > > >> >> >> > > > > >> >> > > >> >> >> +enum rte_max_simd_t { +RTE_NO_SIMD = 64, +RTE_MAX_128_SIMD = > > >> >> >> 128, +RTE_MAX_256_SIMD = 256, +RTE_MAX_512_SIMD = 512, > > >> >> >> +RTE_MAX_SIMD_DISABLE = UINT16_MAX, }; > > >> >> > > > >> >> >What is the difference between RTE_NO_SIMD and > > >> >RTE_MAX_SIMD_DISABLE? > > >> >> > > >> >> RTE_NO_SIMD has value 64 to limit paths to scalar only. > > >> >> RTE_MAX_SIMD_DISABLE sets the highest value possible, so > > >> >> essentially disables the limit affecting which vector paths are > > >> >> taken. This disable option was added to allow for ARM SVE which > > >> >> will be later added, Discussed with Honnappa on a previous version: > > >> >> https://patchwork.dpdk.org/patch/76097/ > > >> > > > >> >Ok, so RTE_MAX_SIMD_DISABLE means "disable the max limit", right? > > >> > > > >> >I feel the name is a bit confusing. What about something like this: > > >> > > > >> >enum rte_simd { RTE_SIMD_DISABLED = 0, RTE_SIMD_128 = 128, > > >> >RTE_SIMD_256 = 256, RTE_SIMD_512 = 512, RTE_SIMD_MAX = UINT16_MAX, }; > > >> > > > >> > > > >> > > >> Sure, I can rename these. Although will implement with > > >RTE_SIMD_DISABLED=64 to allow for scalar path only. > > > > > >Out of curiosity, why 64? I thought 0 was a good value for "disabled". > > > > > > > 64 was chosen because it represents the max bitwidth for the scalar path, > > 64 bits. Currently, we use 0 on the command-line to represent the > > RTE_SIMD_MAX = UINT16_MAX, as it is more user friendly to pass > > "--force-max-simd-bitwidth=0" rather than a max value, the 0 is then > > internally converted to the max value option. This would not be possible > > if we have the scalar option as 0 value. > > > > >> >> > > >> >> >The default value in internal_config is 0, so in my understanding > > >> >> >rte_get_max_simd_bitwidth() will return 0 if > > >> >> >--force-max-simd-bitwidth is not passed. Is it expected? > > >> >> > > > >> >> >Maybe I'm missing something, but I don't understand why the value > > >> >> >in internal_config is not set to the maximum supported SIMD > > >> >> >bitwidth by default, and optionally overriden by the command line > > >> >> >argument, or by the API. > > >> >> > > > >> >> > > >> >> The default value for max_simd_bitwidth is set depending on the > > >> >> architecture, 256 for x86/ppc, and UINT16_MAX for ARM. So for > > >> >> example > > >> >the default on x86 allows for AVX2 and under. > > >> >> The defaults can be seen in patch 2: > > >> >> https://patchwork.dpdk.org/patch/79339/ > > >> > > > >> >Ok, I was expecting to have a runtime check for this. For instance, > > >> >on intel architecture, it is not known at compilation, it depends on > > >> >the target which can support up to AVX, AVX2, or AVX512. > > >> > > > >> > > >> Yes, the actual support will vary, but this max SIMD bitwidth is only > > >> an > > >upper limit on what paths can be taken. > > >> So for example with x86 default at 256, the path will still be chosen > > >> based > > >on what the target can support, but it must be AVX2 or a lesser path. > > >> This allows for AVX512 to be enabled at runtime, by increasing the max > > >SIMD bitwidth to 512, allowing for that path to be taken where > > >supported. > > > > > >Ah, this means that AVX512 won't be enabled by default on machine that > > >support it? Is there a reason for that? > > > > > > > We can't enable the AVX512 by default because it can cause CPU frequency > > slowdowns, But this will allow runtime enabling to take that path if the > > app/user finds it is the best choice for their use, by setting the max > > SIMD bitwidth to 512. > > > > >Another question: if the default value for max-simd-bitwidth is 256 on > > >Intel, and we are running on a target that does not support AVX2, will > > >the value be updated to 128 at initialization? In other word, is it > > >still up to the dpdk libraries doing vector code to check the > > >availability of vector instructions? > > > > > >Thanks, Olivier > > > > No the value is not updated depending on the support, it is just a limit. > > Libraries still do the checks they had done previously to check what is > > supported, and once that supported path is within the max SIMD bitwidth > > limit, it is okay to go ahead, otherwise it will need to choose a lesser > > path. For example, if a library supports AVX2, SSE and a scalar path, > > but the max SIMD bitwidth is set at 128 by app/user, although the library > > supports AVX2, it will be limited to choosing the SSE path. Whereas if > > for example a library supports only SSE and a scalar path, and the > > default max SIMD bitwidth is used (256), the library can still choose SSE > > as it is below the 256 bit limit, and the limit remains at 256. > > > Just to note too that the reason for keeping this separation is that the > actual code path selection in each library is going to have to be > architecture specific, e.g. to choose SSE or NEON for 128-bit width, > whether or not the max-bitwidth functions take the running system into > account. By leaving the libs/drivers to check the CPU support themselves, > it keeps the max-bitwidth functions generic and saves having > architecture-specific code in two places for this. Ok, that's clearer to me, thanks Ciara and Bruce.