| Message ID | 20180731163059.27085-1-stephen@networkplumber.org (mailing list archive) |
|---|---|
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5AD861B05; Tue, 31 Jul 2018 18:31:04 +0200 (CEST) Received: from mail-pf1-f195.google.com (mail-pf1-f195.google.com [209.85.210.195]) by dpdk.org (Postfix) with ESMTP id 7AD0CF72 for <dev@dpdk.org>; Tue, 31 Jul 2018 18:31:03 +0200 (CEST) Received: by mail-pf1-f195.google.com with SMTP id k21-v6so6365700pff.11 for <dev@dpdk.org>; Tue, 31 Jul 2018 09:31:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=aFHTJAml9yTQPm8W56CevFBuzplnkiVqCM38xR7Hmxc=; b=hEyr97wQ6wVB/IVpN+CMByLUO5bM3QgClzJrbnLRl7mcf/r+ieh1HZdaIY1We/hTOW 8c4u8+yqUTmvDk1FhorM1HuyJtu4zVyJYy1ZqJvon9STKEsddQGd8yJu+hMjYOCZ0rqX xO1zfQJcXYzeMnmV9iwXT1JLqKT4Y6XMdXzwWIj8LH4NAhPDt59o4cAyHUYIZ1Dgp2gH CLDJT8jIINGLD1ut6k9jU1Y1kk/yPN7Zesy5nNyHgF0oU0jiDoAqEh386iUkXGqgYqTC weFhtjYdkP4vb1FlD55GnERnB5JdqGTPNUwVhBU6kfzfYDH0iPnbYmXSguoqLcotDNRo y7oQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=aFHTJAml9yTQPm8W56CevFBuzplnkiVqCM38xR7Hmxc=; b=qJl907RlktxmC3nun8LZBgAxBiyvH54BeiGYwlR9uZ4pB6FTufp6ye1FNeSqkw49xR pPw40CrcK5oVVH3vA3ktPNcYRyp6Bh5YQ2ozZRruwirirFbZHZQRzDude9teExnTPGBN v9ZIFGqEFh4HKCnkD93NNfJizVqW6WwuKhQMkdLsGmYsyhD3M1L6UarPrMW8ydNMvih2 BNC9caa5WBtwrsaAmm+L1X6E/H1bQvXwmMn0mFmObzw9K3hy0AN5Fh0DQU/xWx2XNxAS CuRxlq+jLaCMokcTP70KzKXLJWNJS+X+GjofkIsisPVYoIKTx2jzhUygd1CfFc+lhDmq 5cLQ== X-Gm-Message-State: AOUpUlHKuE9hs7c8p/H+8Wcpbr5Be/Oy8vVPQD/Eh6BIBZ3PTVSlYKU0 uhhdUIAXNt8DXiLNayJeuCzc5xtoWTg= X-Google-Smtp-Source: AAOMgpezJhWJ5pw29P1B3NiXdtHuDZcVGvMNAoPbtws7Se+I4dYXJeYt7N2d021qv2CR7ODpjMtzVQ== X-Received: by 2002:a65:5286:: with SMTP id y6-v6mr21188974pgp.65.1533054662345; Tue, 31 Jul 2018 09:31:02 -0700 (PDT) Received: from xeon-e3.wavecable.com (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id g184-v6sm20661619pgc.22.2018.07.31.09.31.01 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 31 Jul 2018 09:31:01 -0700 (PDT) From: Stephen Hemminger <stephen@networkplumber.org> To: dev@dpdk.org Cc: Stephen Hemminger <stephen@networkplumber.org> Date: Tue, 31 Jul 2018 09:30:54 -0700 Message-Id: <20180731163059.27085-1-stephen@networkplumber.org> X-Mailer: git-send-email 2.18.0 Subject: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
| Series | remove usage of register keyword in C | |
Message
Stephen Hemminger
July 31, 2018, 4:30 p.m. UTC
Declaring variables as register in C is a leftover from an earlier era (like cassette tape decks in cars). Stephen Hemminger (5): qat: remove redundant C register keyword qede: remove register from declaraitons ark: remove register keyword mlx5: no need for register keyword mlx4: remove redunant register keyword drivers/common/qat/qat_qp.c | 10 +++++----- drivers/crypto/qat/qat_sym.c | 2 +- drivers/net/ark/ark_ethdev_rx.c | 4 ++-- drivers/net/mlx4/mlx4_mr.c | 2 +- drivers/net/mlx5/mlx5_mr.c | 2 +- drivers/net/qede/qede_rxtx.c | 8 ++++---- 6 files changed, 14 insertions(+), 14 deletions(-)
Comments
On Tue, Jul 31, 2018 at 09:30:54AM -0700, Stephen Hemminger wrote: > Declaring variables as register in C is a leftover from an earlier > era (like cassette tape decks in cars). I don't agree here. It's a hint for compilers and developers that the address of such variables won't be needed (and cannot be taken) to enable whatever optimizations are possible knowing this. Somewhat like inline functions, it's not a forced optimization, just a useful hint that shouldn't hurt if used wisely. Besides, cassette decks are not dead yet :)
On Tue, 31 Jul 2018 18:48:40 +0200 Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote: > On Tue, Jul 31, 2018 at 09:30:54AM -0700, Stephen Hemminger wrote: > > Declaring variables as register in C is a leftover from an earlier > > era (like cassette tape decks in cars). > > I don't agree here. It's a hint for compilers and developers that the > address of such variables won't be needed (and cannot be taken) to enable > whatever optimizations are possible knowing this. > > Somewhat like inline functions, it's not a forced optimization, just a > useful hint that shouldn't hurt if used wisely. > > Besides, cassette decks are not dead yet :) If you look at the code, that is not how register is being used (ie. don't take address of this). It seems like an attempt at optimization.
Hi Stephen Can you elaborate more? Can you add references? > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger > Sent: Tuesday, July 31, 2018 7:31 PM > To: dev@dpdk.org > Cc: Stephen Hemminger <stephen@networkplumber.org> > Subject: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C > > Declaring variables as register in C is a leftover from an earlier era (like > cassette tape decks in cars). > > Stephen Hemminger (5): > qat: remove redundant C register keyword > qede: remove register from declaraitons > ark: remove register keyword > mlx5: no need for register keyword > mlx4: remove redunant register keyword > > drivers/common/qat/qat_qp.c | 10 +++++----- > drivers/crypto/qat/qat_sym.c | 2 +- > drivers/net/ark/ark_ethdev_rx.c | 4 ++-- > drivers/net/mlx4/mlx4_mr.c | 2 +- > drivers/net/mlx5/mlx5_mr.c | 2 +- > drivers/net/qede/qede_rxtx.c | 8 ++++---- > 6 files changed, 14 insertions(+), 14 deletions(-) > > -- > 2.18.0
> On Jul 31, 2018, at 11:07 AM, Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Tue, 31 Jul 2018 18:48:40 +0200 > Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote: > >> On Tue, Jul 31, 2018 at 09:30:54AM -0700, Stephen Hemminger wrote: >>> Declaring variables as register in C is a leftover from an earlier >>> era (like cassette tape decks in cars). >> >> I don't agree here. It's a hint for compilers and developers that the >> address of such variables won't be needed (and cannot be taken) to enable >> whatever optimizations are possible knowing this. >> >> Somewhat like inline functions, it's not a forced optimization, just a >> useful hint that shouldn't hurt if used wisely. >> >> Besides, cassette decks are not dead yet :) > > If you look at the code, that is not how register is being used (ie. don't take > address of this). It seems like an attempt at optimization. I know compilers are smart enough and the occurrences in mlx4/5 were made from my old fashioned habit. But, I don't see any urgency to push this patch in RC stage even though I'm 99% sure that it is harmless. And in general I don't even understand why we can't live with that if it isn't harmful (or a violation) but informative. I mean no badness but at least one goodness :-) Thanks, Yongseok
On Wed, 1 Aug 2018 18:03:04 +0000 Yongseok Koh <yskoh@mellanox.com> wrote: > > On Jul 31, 2018, at 11:07 AM, Stephen Hemminger <stephen@networkplumber.org> wrote: > > > > On Tue, 31 Jul 2018 18:48:40 +0200 > > Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote: > > > >> On Tue, Jul 31, 2018 at 09:30:54AM -0700, Stephen Hemminger wrote: > >>> Declaring variables as register in C is a leftover from an earlier > >>> era (like cassette tape decks in cars). > >> > >> I don't agree here. It's a hint for compilers and developers that the > >> address of such variables won't be needed (and cannot be taken) to enable > >> whatever optimizations are possible knowing this. > >> > >> Somewhat like inline functions, it's not a forced optimization, just a > >> useful hint that shouldn't hurt if used wisely. > >> > >> Besides, cassette decks are not dead yet :) > > > > If you look at the code, that is not how register is being used (ie. don't take > > address of this). It seems like an attempt at optimization. > > I know compilers are smart enough and the occurrences in mlx4/5 were made from > my old fashioned habit. But, I don't see any urgency to push this patch in RC > stage even though I'm 99% sure that it is harmless. And in general I don't even > understand why we can't live with that if it isn't harmful (or a violation) but > informative. I mean no badness but at least one goodness :-) > > Thanks, > Yongseok > Sure, this is intended for next release not rc stage. Just trying to clean up code base where I see it.
On 8/1/2018 10:03 PM, Stephen Hemminger wrote: > On Wed, 1 Aug 2018 18:03:04 +0000 > Yongseok Koh <yskoh@mellanox.com> wrote: > >>> On Jul 31, 2018, at 11:07 AM, Stephen Hemminger <stephen@networkplumber.org> wrote: >>> >>> On Tue, 31 Jul 2018 18:48:40 +0200 >>> Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote: >>> >>>> On Tue, Jul 31, 2018 at 09:30:54AM -0700, Stephen Hemminger wrote: >>>>> Declaring variables as register in C is a leftover from an earlier >>>>> era (like cassette tape decks in cars). >>>> >>>> I don't agree here. It's a hint for compilers and developers that the >>>> address of such variables won't be needed (and cannot be taken) to enable >>>> whatever optimizations are possible knowing this. >>>> >>>> Somewhat like inline functions, it's not a forced optimization, just a >>>> useful hint that shouldn't hurt if used wisely. >>>> >>>> Besides, cassette decks are not dead yet :) >>> >>> If you look at the code, that is not how register is being used (ie. don't take >>> address of this). It seems like an attempt at optimization. >> >> I know compilers are smart enough and the occurrences in mlx4/5 were made from >> my old fashioned habit. But, I don't see any urgency to push this patch in RC >> stage even though I'm 99% sure that it is harmless. And in general I don't even >> understand why we can't live with that if it isn't harmful (or a violation) but >> informative. I mean no badness but at least one goodness :-) >> >> Thanks, >> Yongseok >> > > Sure, this is intended for next release not rc stage. > Just trying to clean up code base where I see it. I agree with Yongseok, at worst they show the intention of the developer, I don't see motivation to remove them unless they are doing something wrong, which seems not the reason of this patch. And although I found some information that says "register" ignored completely for gcc, I can see it differs when optimization disabled. I am not saying practically it differs, since we enable optimization expect from debugging, most probably there is no practical difference between having the keyword or not, but what I am trying to say is it not completely ignored either.
On 8/23/2018 2:07 PM, Ferruh Yigit wrote: > On 8/1/2018 10:03 PM, Stephen Hemminger wrote: >> On Wed, 1 Aug 2018 18:03:04 +0000 >> Yongseok Koh <yskoh@mellanox.com> wrote: >> >>>> On Jul 31, 2018, at 11:07 AM, Stephen Hemminger <stephen@networkplumber.org> wrote: >>>> >>>> On Tue, 31 Jul 2018 18:48:40 +0200 >>>> Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote: >>>> >>>>> On Tue, Jul 31, 2018 at 09:30:54AM -0700, Stephen Hemminger wrote: >>>>>> Declaring variables as register in C is a leftover from an earlier >>>>>> era (like cassette tape decks in cars). >>>>> >>>>> I don't agree here. It's a hint for compilers and developers that the >>>>> address of such variables won't be needed (and cannot be taken) to enable >>>>> whatever optimizations are possible knowing this. >>>>> >>>>> Somewhat like inline functions, it's not a forced optimization, just a >>>>> useful hint that shouldn't hurt if used wisely. >>>>> >>>>> Besides, cassette decks are not dead yet :) >>>> >>>> If you look at the code, that is not how register is being used (ie. don't take >>>> address of this). It seems like an attempt at optimization. >>> >>> I know compilers are smart enough and the occurrences in mlx4/5 were made from >>> my old fashioned habit. But, I don't see any urgency to push this patch in RC >>> stage even though I'm 99% sure that it is harmless. And in general I don't even >>> understand why we can't live with that if it isn't harmful (or a violation) but >>> informative. I mean no badness but at least one goodness :-) >>> >>> Thanks, >>> Yongseok >>> >> >> Sure, this is intended for next release not rc stage. >> Just trying to clean up code base where I see it. > > I agree with Yongseok, at worst they show the intention of the developer, I > don't see motivation to remove them unless they are doing something wrong, which > seems not the reason of this patch. > > And although I found some information that says "register" ignored completely > for gcc, I can see it differs when optimization disabled. > I am not saying practically it differs, since we enable optimization expect from > debugging, most probably there is no practical difference between having the > keyword or not, but what I am trying to say is it not completely ignored either. I am for marking this set as rejected, any objection?
Hi all, Has there been any change regarding this? I'm still at DPDK 18.11. Maybe automatically add -Wno-register when C++17 is enabled? Or have a some register macro which gets undefined if C++17 is enabled? The "warning: ISO C++1z does not allow ‘register’ storage class specifier" is annoying. And vim always goes to some DPDK header when ":make" fails because of the warning... Thanks, Tom On 2018-10-09 11:19, Ferruh Yigit wrote: > On 8/23/2018 2:07 PM, Ferruh Yigit wrote: >> On 8/1/2018 10:03 PM, Stephen Hemminger wrote: >>> On Wed, 1 Aug 2018 18:03:04 +0000 >>> Yongseok Koh <yskoh@mellanox.com> wrote: >>> >>>>> On Jul 31, 2018, at 11:07 AM, Stephen Hemminger <stephen@networkplumber.org> wrote: >>>>> >>>>> On Tue, 31 Jul 2018 18:48:40 +0200 >>>>> Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote: >>>>> >>>>>> On Tue, Jul 31, 2018 at 09:30:54AM -0700, Stephen Hemminger wrote: >>>>>>> Declaring variables as register in C is a leftover from an earlier >>>>>>> era (like cassette tape decks in cars). >>>>>> >>>>>> I don't agree here. It's a hint for compilers and developers that the >>>>>> address of such variables won't be needed (and cannot be taken) to enable >>>>>> whatever optimizations are possible knowing this. >>>>>> >>>>>> Somewhat like inline functions, it's not a forced optimization, just a >>>>>> useful hint that shouldn't hurt if used wisely. >>>>>> >>>>>> Besides, cassette decks are not dead yet :) >>>>> >>>>> If you look at the code, that is not how register is being used (ie. don't take >>>>> address of this). It seems like an attempt at optimization. >>>> >>>> I know compilers are smart enough and the occurrences in mlx4/5 were made from >>>> my old fashioned habit. But, I don't see any urgency to push this patch in RC >>>> stage even though I'm 99% sure that it is harmless. And in general I don't even >>>> understand why we can't live with that if it isn't harmful (or a violation) but >>>> informative. I mean no badness but at least one goodness :-) >>>> >>>> Thanks, >>>> Yongseok >>>> >>> >>> Sure, this is intended for next release not rc stage. >>> Just trying to clean up code base where I see it. >> >> I agree with Yongseok, at worst they show the intention of the developer, I >> don't see motivation to remove them unless they are doing something wrong, which >> seems not the reason of this patch. >> >> And although I found some information that says "register" ignored completely >> for gcc, I can see it differs when optimization disabled. >> I am not saying practically it differs, since we enable optimization expect from >> debugging, most probably there is no practical difference between having the >> keyword or not, but what I am trying to say is it not completely ignored either. > > I am for marking this set as rejected, any objection? >
On Thu, Jan 31, 2019 at 09:02:36AM +0100, Tom Barbette wrote: > Hi all, > > Has there been any change regarding this? I'm still at DPDK 18.11. Maybe > automatically add -Wno-register when C++17 is enabled? Or have a some > register macro which gets undefined if C++17 is enabled? > > The "warning: ISO C++1z does not allow ‘register’ storage class specifier" > is annoying. And vim always goes to some DPDK header when ":make" fails > because of the warning... > > Thanks, > Tom > What header is that? From what I see the patchset only makes changes to .c files rather than any .h files, so not sure it would help in your case. For what it's worth on the general discussion, I'm in favour of applying this patchset. I view marking variables as "register" as completely unncessary. If someone can demonstrate a place where it actually makes a difference, then we can keep that use of the keywork, otherwise I think the code is as well off without it. /Bruce
On 2019-01-31 10:11, Bruce Richardson wrote: > What header is that? From what I see the patchset only makes changes to .c > files rather than any .h files, so not sure it would help in your case. Yes you're right. There are other occurrences of register indeed. I got the warning from rte_common.h:308 : rte_combine64ms1b(register uint64_t v) but it may not be the only one? Tom
I agree using register for todays compilers is unnecessary and can actually be wrong in some cases. The compilers can pick the correct registers better then we can normally and restricting the compiler makes no sense. Sent from my iPhone > On Jan 31, 2019, at 3:11 AM, Bruce Richardson <bruce.richardson@intel.com> wrote: > >> On Thu, Jan 31, 2019 at 09:02:36AM +0100, Tom Barbette wrote: >> Hi all, >> >> Has there been any change regarding this? I'm still at DPDK 18.11. Maybe >> automatically add -Wno-register when C++17 is enabled? Or have a some >> register macro which gets undefined if C++17 is enabled? >> >> The "warning: ISO C++1z does not allow ‘register’ storage class specifier" >> is annoying. And vim always goes to some DPDK header when ":make" fails >> because of the warning... >> >> Thanks, >> Tom >> > What header is that? From what I see the patchset only makes changes to .c > files rather than any .h files, so not sure it would help in your case. > > For what it's worth on the general discussion, I'm in favour of applying > this patchset. I view marking variables as "register" as completely > unncessary. If someone can demonstrate a place where it actually makes a > difference, then we can keep that use of the keywork, otherwise I think the > code is as well off without it. > > /Bruce