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 28DB2A051C; Fri, 26 Jun 2020 16:37:41 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A97B31C0AF; Fri, 26 Jun 2020 16:37:40 +0200 (CEST) Received: from mail-il1-f172.google.com (mail-il1-f172.google.com [209.85.166.172]) by dpdk.org (Postfix) with ESMTP id 18B371C08C for ; Fri, 26 Jun 2020 16:37:39 +0200 (CEST) Received: by mail-il1-f172.google.com with SMTP id t8so8657889ilm.7 for ; Fri, 26 Jun 2020 07:37:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=Nl3i6sf5K1iMWcgl0KXbjRhFx058bsRfkvOFc+Bkuuw=; b=Ff2OtsgX+TZxgzWJuXycQ5Tag7mM2mkvStAmZWTg6jPOyro099AV9JXMmZ1hHRxCV/ PfMalYZ+iKBmwpQfW2L+iBEFlpHMaGyCdsJB1HxrXDjZ3UjFLHBla3T9nD1cjeSvQxyq lFtKNt87hRoo98VfNf9Hv3M7nl9/La9VOXNBk9DzLu9Ih2Prgd3bH48T/vKsHKDhlQq3 KNtfIOQUJP/lgz54Ver96jcZIsbrQunNSZdIkMPRCFmr534fshvfUXRwX9OX+U/2qDMB hv2MOWtNGxi9ukwFx5k86S0Kz6hM/vTn/TWAhMlTS6nfbMqEk16rhkPJy9PpE4y1cd+L U5GQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=Nl3i6sf5K1iMWcgl0KXbjRhFx058bsRfkvOFc+Bkuuw=; b=ZRk9OA8VBtyvFeXg8ahl1fusOz2QRFhj+/iuUsrOxSe5xsI1Qwz+NcsfxsQMz7zLUB qGWnPkXpfMoFbhGuSXCiHBHXbfhtzU463dTr+S3YNus8pmgiaqYw/OM8+xL5yWY3nsWn 8YI9yWfUBoCtaLoVbc69xxrxLSf0xSYlZHpgzVtl9xBPvaj/su76OGO7wXXWzUZefcqm JmVp5ZsDoTvQSb8aQuO9/J8Eja+kplKjfDkca0iNgKHWr14LdYlo0HIsMrupGM+4mm+k ltTcNF4Cvg86FRALEs/u6JyR8G4HetuOw0kH2mdX9UMddjAooA+WZP/b9tAfAbxZPxmY Xo6A== X-Gm-Message-State: AOAM5326CusLpXSRIS8FoHXVS0VyddztsNx9BAcQYZ3MfQhq9OE2ka9H P8NY87W49790qgRyEVD8WiPOTEUvX6jjxfBUBHT8GguL/3nkEQ== X-Google-Smtp-Source: ABdhPJz2eu+BG5i1zXjam88ZK198VQomdlwhp+PkUF1oFaf3pGxnSY1aBb6I2zuufcOIb/lFZT23CK3JmBWXpASdpxs= X-Received: by 2002:a92:d01:: with SMTP id 1mr2526435iln.294.1593182258264; Fri, 26 Jun 2020 07:37:38 -0700 (PDT) MIME-Version: 1.0 References: <98CBD80474FA8B44BF855DF32C47DC35C610C4@smartserver.smartshare.dk> <6b67ce84-92ee-550d-2fba-af8c4c1bb2aa@intel.com> <98CBD80474FA8B44BF855DF32C47DC35C610C7@smartserver.smartshare.dk> In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35C610C7@smartserver.smartshare.dk> From: Jerin Jacob Date: Fri, 26 Jun 2020 20:07:22 +0530 Message-ID: To: =?UTF-8?Q?Morten_Br=C3=B8rup?= Cc: Ferruh Yigit , dpdk-dev , Olivier Matz , Harry Van Haaren , Konstantin Ananyev Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] rte_ether_addr_copy() strange comment 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 Fri, Jun 26, 2020 at 6:05 PM Morten Br=C3=B8rup wrote: > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit > > Sent: Friday, June 26, 2020 2:08 PM > > > > On 6/25/2020 4:45 PM, Morten Br=C3=B8rup wrote: > > > The function rte_ether_addr_copy() checks for __INTEL_COMPILER and > > has a comment about "a strange gcc warning". It says: > > > > > > static inline void rte_ether_addr_copy(const struct rte_ether_addr > > *ea_from, > > > struct rte_ether_addr *ea_to) > > > { > > > #ifdef __INTEL_COMPILER > > > uint16_t *from_words =3D (uint16_t *)(ea_from->addr_bytes); > > > uint16_t *to_words =3D (uint16_t *)(ea_to->addr_bytes); > > > > > > to_words[0] =3D from_words[0]; > > > to_words[1] =3D from_words[1]; > > > to_words[2] =3D from_words[2]; > > > #else > > > /* > > > * Use the common way, because of a strange gcc warning. > > > */ > > > *ea_to =3D *ea_from; > > > #endif > > > } > > > > > > I can see that from_words discards the const qualifier. Is that the > > "strange" gcc warning the comment is referring to? > > > > > > This goes back to before the first public release of DPDK in 2013, > > ref. https://git.dpdk.org/dpdk/log/lib/librte_ether/rte_ether.h > > > > > > > > > It should be fixed as follows: > > > > > > - uint16_t *from_words =3D (uint16_t *)(ea_from->addr_bytes); > > > - uint16_t *to_words =3D (uint16_t *)(ea_to->addr_bytes); > > > + const uint16_t *from_words =3D (const uint16_t *)ea_from; > > > + uint16_t *to_words =3D (uint16_t *)ea_to; > > > > > > And the consequential question: Is copying the three shorts faster > > than copying the struct? In other words: Should we get rid of the > > #ifdef and use the first method only? > > > > > > I tried to investigate this in godbolt: https://godbolt.org/z/YSmvDn > > > > First I don't see the "strange" gcc warning with various gcc versions > > there. > > > > Related to the struct copy vs word copy, struct copy seems with less > > instruction > > [1],[2], > > my vote to remove ifdef and keep struct copy. > > > > > > [1] copy as individual function > > [1a] gcc 10.1, struct copy: > > copy: > > movdqa (%rsi), %xmm0 > > movaps %xmm0, (%rdi) > > ret > > > > [1b] gcc 10.1, word copy: > > copy: > > movzwl (%rsi), %eax > > movw %ax, (%rdi) > > movzwl 2(%rsi), %eax > > movw %ax, 2(%rdi) > > movzwl 4(%rsi), %eax > > movw %ax, 4(%rdi) > > ret > > > > [1c] icc 19.0.1, struct copy > > copy: > > movups (%rsi), %xmm0 #19.13 > > movups %xmm0, (%rdi) #19.13 > > ret > > > > > > [2] gcc 10.1, copy as inline function that knows the data, both seems > > similar > > // .addr =3D {1, 1, 1, 1, 1, 1}, > > [2a] struct copy: > > ... > > movl $257, %eax > > movw %ax, 4(%rsp) > > leaq 16(%rsp), %rdi > > movl $16843009, (%rsp) > > movdqa (%rsp), %xmm0 > > movaps %xmm0, 16(%rsp) > > ... > > > > [2b] word copy: > > movl $257, %eax > > movq %rsp, %rdi > > movw %ax, 4(%rsp) > > movl $16843009, (%rsp) > > > > Thank you for the detailed response, Ferruh. > > I didn't know about godbolt, so thank you for that reference too. > > The address struct is 2 byte aligned, not 16 byte aligned. Modifying your= test in godbolt to use 2 byte alignment gives a similar result, i.e. fewer= instructions on both icc and gcc. > > [1c-modified] icc 19.0.1, struct copy > > copy: > movl (%rsi), %eax #19.13 > movl %eax, (%rdi) #19.13 > movzwl 4(%rsi), %edx #19.13 > movw %dx, 4(%rdi) #19.13 > ret #28.1 > > [1d-modified] icc 19.0.1, word copy > copy: > movzwl (%rsi), %eax #24.12 > movw %ax, (%rdi) #24.5 > movzwl 2(%rsi), %edx #25.12 > movw %dx, 2(%rdi) #25.5 > movzwl 4(%rsi), %ecx #26.12 > movw %cx, 4(%rdi) #26.5 > ret #28.1 > > Testing for ARM64 on godbolt gives a similar result: more instructions us= ing word copy than struct copy. > > In conclusion, I will proceed with the struct copy. Since you are up to changing the code, Could you add __restrict keyword for the further hint to the compiler for struct copy case? http://mails.dpdk.org/archives/dev/2020-June/169876.html