DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C
@ 2018-07-31 16:30 Stephen Hemminger
  2018-07-31 16:30 ` [dpdk-dev] [PATCH 1/5] qat: remove redundant C register keyword Stephen Hemminger
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Stephen Hemminger @ 2018-07-31 16:30 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 1/5] qat: remove redundant C register keyword
  2018-07-31 16:30 [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C Stephen Hemminger
@ 2018-07-31 16:30 ` Stephen Hemminger
  2018-09-05 10:23   ` Jozwiak, TomaszX
  2018-07-31 16:30 ` [dpdk-dev] [PATCH 2/5] qede: remove register from declaraitons Stephen Hemminger
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2018-07-31 16:30 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Modern C compilers ignore the register keyword and do automatic
register assignment.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/common/qat/qat_qp.c  | 10 +++++-----
 drivers/crypto/qat/qat_sym.c |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/common/qat/qat_qp.c b/drivers/common/qat/qat_qp.c
index 7ca7a45ebfe0..21891ec1955d 100644
--- a/drivers/common/qat/qat_qp.c
+++ b/drivers/common/qat/qat_qp.c
@@ -530,13 +530,13 @@ void rxq_free_desc(struct qat_qp *qp, struct qat_queue *q)
 uint16_t
 qat_enqueue_op_burst(void *qp, void **ops, uint16_t nb_ops)
 {
-	register struct qat_queue *queue;
+	struct qat_queue *queue;
 	struct qat_qp *tmp_qp = (struct qat_qp *)qp;
-	register uint32_t nb_ops_sent = 0;
-	register int ret;
+	uint32_t nb_ops_sent = 0;
+	int ret;
 	uint16_t nb_ops_possible = nb_ops;
-	register uint8_t *base_addr;
-	register uint32_t tail;
+	uint8_t *base_addr;
+	uint32_t tail;
 	int overflow;
 
 	if (unlikely(nb_ops == 0))
diff --git a/drivers/crypto/qat/qat_sym.c b/drivers/crypto/qat/qat_sym.c
index 8273968f3aa7..dc77ca870fc9 100644
--- a/drivers/crypto/qat/qat_sym.c
+++ b/drivers/crypto/qat/qat_sym.c
@@ -150,7 +150,7 @@ qat_sym_build_request(void *in_op, uint8_t *out_msg,
 	struct qat_sym_session *ctx;
 	struct icp_qat_fw_la_cipher_req_params *cipher_param;
 	struct icp_qat_fw_la_auth_req_params *auth_param;
-	register struct icp_qat_fw_la_bulk_req *qat_req;
+	struct icp_qat_fw_la_bulk_req *qat_req;
 	uint8_t do_auth = 0, do_cipher = 0, do_aead = 0;
 	uint32_t cipher_len = 0, cipher_ofs = 0;
 	uint32_t auth_len = 0, auth_ofs = 0;
-- 
2.18.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 2/5] qede: remove register from declaraitons
  2018-07-31 16:30 [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C Stephen Hemminger
  2018-07-31 16:30 ` [dpdk-dev] [PATCH 1/5] qat: remove redundant C register keyword Stephen Hemminger
@ 2018-07-31 16:30 ` Stephen Hemminger
  2018-07-31 16:30 ` [dpdk-dev] [PATCH 3/5] ark: remove register keyword Stephen Hemminger
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2018-07-31 16:30 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Modern C compilers do register assignment automatically.
Remove redundant register modifier.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/qede/qede_rxtx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c
index 0f157ded23c8..2be7699d60df 100644
--- a/drivers/net/qede/qede_rxtx.c
+++ b/drivers/net/qede/qede_rxtx.c
@@ -1245,8 +1245,8 @@ qede_process_sg_pkts(void *p_rxq,  struct rte_mbuf *rx_mb,
 {
 	struct qede_rx_queue *rxq = p_rxq;
 	struct qede_dev *qdev = rxq->qdev;
-	register struct rte_mbuf *seg1 = NULL;
-	register struct rte_mbuf *seg2 = NULL;
+	struct rte_mbuf *seg1 = NULL;
+	struct rte_mbuf *seg2 = NULL;
 	uint16_t sw_rx_index;
 	uint16_t cur_size;
 
@@ -1305,8 +1305,8 @@ qede_recv_pkts(void *p_rxq, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	uint16_t rx_pkt = 0;
 	union eth_rx_cqe *cqe;
 	struct eth_fast_path_rx_reg_cqe *fp_cqe = NULL;
-	register struct rte_mbuf *rx_mb = NULL;
-	register struct rte_mbuf *seg1 = NULL;
+	struct rte_mbuf *rx_mb = NULL;
+	struct rte_mbuf *seg1 = NULL;
 	enum eth_rx_cqe_type cqe_type;
 	uint16_t pkt_len = 0; /* Sum of all BD segments */
 	uint16_t len; /* Length of first BD */
-- 
2.18.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 3/5] ark: remove register keyword
  2018-07-31 16:30 [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C Stephen Hemminger
  2018-07-31 16:30 ` [dpdk-dev] [PATCH 1/5] qat: remove redundant C register keyword Stephen Hemminger
  2018-07-31 16:30 ` [dpdk-dev] [PATCH 2/5] qede: remove register from declaraitons Stephen Hemminger
@ 2018-07-31 16:30 ` Stephen Hemminger
  2018-07-31 16:30 ` [dpdk-dev] [PATCH 4/5] mlx5: no need for " Stephen Hemminger
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2018-07-31 16:30 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

No need for register keyword with modern compilers.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/ark/ark_ethdev_rx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ark/ark_ethdev_rx.c b/drivers/net/ark/ark_ethdev_rx.c
index 16f0d11ecac5..19e10955103e 100644
--- a/drivers/net/ark/ark_ethdev_rx.c
+++ b/drivers/net/ark/ark_ethdev_rx.c
@@ -236,7 +236,7 @@ eth_ark_recv_pkts(void *rx_queue,
 		  uint16_t nb_pkts)
 {
 	struct ark_rx_queue *queue;
-	register uint32_t cons_index, prod_index;
+	uint32_t cons_index, prod_index;
 	uint16_t nb;
 	struct rte_mbuf *mbuf;
 	struct ark_rx_meta *meta;
@@ -364,7 +364,7 @@ eth_ark_rx_jumbo(struct ark_rx_queue *queue,
 static void
 eth_ark_rx_queue_drain(struct ark_rx_queue *queue)
 {
-	register uint32_t cons_index;
+	uint32_t cons_index;
 	struct rte_mbuf *mbuf;
 
 	cons_index = queue->cons_index;
-- 
2.18.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 4/5] mlx5: no need for register keyword
  2018-07-31 16:30 [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C Stephen Hemminger
                   ` (2 preceding siblings ...)
  2018-07-31 16:30 ` [dpdk-dev] [PATCH 3/5] ark: remove register keyword Stephen Hemminger
@ 2018-07-31 16:30 ` Stephen Hemminger
  2018-07-31 16:30 ` [dpdk-dev] [PATCH 5/5] mlx4: remove redunant " Stephen Hemminger
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2018-07-31 16:30 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The register keyword is redundant with modern compilers.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/mlx5/mlx5_mr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c
index 1d1bcb5fe028..b8fb6c0b9f0d 100644
--- a/drivers/net/mlx5/mlx5_mr.c
+++ b/drivers/net/mlx5/mlx5_mr.c
@@ -104,7 +104,7 @@ mr_btree_lookup(struct mlx5_mr_btree *bt, uint16_t *idx, uintptr_t addr)
 			       lkp_tbl[0].lkey == UINT32_MAX));
 	/* Binary search. */
 	do {
-		register uint16_t delta = n >> 1;
+		uint16_t delta = n >> 1;
 
 		if (addr < lkp_tbl[base + delta].start) {
 			n = delta;
-- 
2.18.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 5/5] mlx4: remove redunant register keyword
  2018-07-31 16:30 [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C Stephen Hemminger
                   ` (3 preceding siblings ...)
  2018-07-31 16:30 ` [dpdk-dev] [PATCH 4/5] mlx5: no need for " Stephen Hemminger
@ 2018-07-31 16:30 ` Stephen Hemminger
  2018-07-31 16:48 ` [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C Adrien Mazarguil
  2018-08-01 10:18 ` Matan Azrad
  6 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2018-07-31 16:30 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/mlx4/mlx4_mr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx4/mlx4_mr.c b/drivers/net/mlx4/mlx4_mr.c
index d23d3c613ea7..6869cfb6e6ef 100644
--- a/drivers/net/mlx4/mlx4_mr.c
+++ b/drivers/net/mlx4/mlx4_mr.c
@@ -120,7 +120,7 @@ mr_btree_lookup(struct mlx4_mr_btree *bt, uint16_t *idx, uintptr_t addr)
 			       lkp_tbl[0].lkey == UINT32_MAX));
 	/* Binary search. */
 	do {
-		register uint16_t delta = n >> 1;
+		uint16_t delta = n >> 1;
 
 		if (addr < lkp_tbl[base + delta].start) {
 			n = delta;
-- 
2.18.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C
  2018-07-31 16:30 [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C Stephen Hemminger
                   ` (4 preceding siblings ...)
  2018-07-31 16:30 ` [dpdk-dev] [PATCH 5/5] mlx4: remove redunant " Stephen Hemminger
@ 2018-07-31 16:48 ` Adrien Mazarguil
  2018-07-31 18:07   ` Stephen Hemminger
  2018-08-01 10:18 ` Matan Azrad
  6 siblings, 1 reply; 18+ messages in thread
From: Adrien Mazarguil @ 2018-07-31 16:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

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 :)

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C
  2018-07-31 16:48 ` [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C Adrien Mazarguil
@ 2018-07-31 18:07   ` Stephen Hemminger
  2018-08-01 18:03     ` Yongseok Koh
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2018-07-31 18:07 UTC (permalink / raw)
  To: Adrien Mazarguil; +Cc: dev

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C
  2018-07-31 16:30 [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C Stephen Hemminger
                   ` (5 preceding siblings ...)
  2018-07-31 16:48 ` [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C Adrien Mazarguil
@ 2018-08-01 10:18 ` Matan Azrad
  6 siblings, 0 replies; 18+ messages in thread
From: Matan Azrad @ 2018-08-01 10:18 UTC (permalink / raw)
  To: Stephen Hemminger, dev

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C
  2018-07-31 18:07   ` Stephen Hemminger
@ 2018-08-01 18:03     ` Yongseok Koh
  2018-08-01 21:03       ` Stephen Hemminger
  0 siblings, 1 reply; 18+ messages in thread
From: Yongseok Koh @ 2018-08-01 18:03 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Adrien Mazarguil, dev


> 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C
  2018-08-01 18:03     ` Yongseok Koh
@ 2018-08-01 21:03       ` Stephen Hemminger
  2018-08-23 13:07         ` Ferruh Yigit
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2018-08-01 21:03 UTC (permalink / raw)
  To: Yongseok Koh; +Cc: Adrien Mazarguil, dev

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C
  2018-08-01 21:03       ` Stephen Hemminger
@ 2018-08-23 13:07         ` Ferruh Yigit
  2018-10-09  9:19           ` Ferruh Yigit
  0 siblings, 1 reply; 18+ messages in thread
From: Ferruh Yigit @ 2018-08-23 13:07 UTC (permalink / raw)
  To: Stephen Hemminger, Yongseok Koh; +Cc: Adrien Mazarguil, dev

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 1/5] qat: remove redundant C register keyword
  2018-07-31 16:30 ` [dpdk-dev] [PATCH 1/5] qat: remove redundant C register keyword Stephen Hemminger
@ 2018-09-05 10:23   ` Jozwiak, TomaszX
  0 siblings, 0 replies; 18+ messages in thread
From: Jozwiak, TomaszX @ 2018-09-05 10:23 UTC (permalink / raw)
  To: Stephen Hemminger, dev


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen
> Hemminger
> Sent: Tuesday, July 31, 2018 6:31 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Subject: [dpdk-dev] [PATCH 1/5] qat: remove redundant C register keyword
> 
> Modern C compilers ignore the register keyword and do automatic register
> assignment.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Tomasz Jozwiak <tomaszx.jozwiak@intel.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C
  2018-08-23 13:07         ` Ferruh Yigit
@ 2018-10-09  9:19           ` Ferruh Yigit
  2019-01-31  8:02             ` Tom Barbette
  0 siblings, 1 reply; 18+ messages in thread
From: Ferruh Yigit @ 2018-10-09  9:19 UTC (permalink / raw)
  To: Stephen Hemminger, Yongseok Koh; +Cc: Adrien Mazarguil, dev

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?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C
  2018-10-09  9:19           ` Ferruh Yigit
@ 2019-01-31  8:02             ` Tom Barbette
  2019-01-31  9:11               ` Bruce Richardson
  0 siblings, 1 reply; 18+ messages in thread
From: Tom Barbette @ 2019-01-31  8:02 UTC (permalink / raw)
  To: Ferruh Yigit, Stephen Hemminger, Yongseok Koh; +Cc: Adrien Mazarguil, dev

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?
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C
  2019-01-31  8:02             ` Tom Barbette
@ 2019-01-31  9:11               ` Bruce Richardson
  2019-01-31  9:39                 ` Tom Barbette
  2019-01-31 17:34                 ` Wiles, Keith
  0 siblings, 2 replies; 18+ messages in thread
From: Bruce Richardson @ 2019-01-31  9:11 UTC (permalink / raw)
  To: Tom Barbette
  Cc: Ferruh Yigit, Stephen Hemminger, Yongseok Koh, Adrien Mazarguil, dev

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C
  2019-01-31  9:11               ` Bruce Richardson
@ 2019-01-31  9:39                 ` Tom Barbette
  2019-01-31 17:34                 ` Wiles, Keith
  1 sibling, 0 replies; 18+ messages in thread
From: Tom Barbette @ 2019-01-31  9:39 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Ferruh Yigit, Stephen Hemminger, Yongseok Koh, Adrien Mazarguil, dev

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C
  2019-01-31  9:11               ` Bruce Richardson
  2019-01-31  9:39                 ` Tom Barbette
@ 2019-01-31 17:34                 ` Wiles, Keith
  1 sibling, 0 replies; 18+ messages in thread
From: Wiles, Keith @ 2019-01-31 17:34 UTC (permalink / raw)
  To: Richardson, Bruce
  Cc: Tom Barbette, Yigit, Ferruh, Stephen Hemminger, Yongseok Koh,
	Adrien Mazarguil, dev

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2019-01-31 17:34 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-31 16:30 [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C Stephen Hemminger
2018-07-31 16:30 ` [dpdk-dev] [PATCH 1/5] qat: remove redundant C register keyword Stephen Hemminger
2018-09-05 10:23   ` Jozwiak, TomaszX
2018-07-31 16:30 ` [dpdk-dev] [PATCH 2/5] qede: remove register from declaraitons Stephen Hemminger
2018-07-31 16:30 ` [dpdk-dev] [PATCH 3/5] ark: remove register keyword Stephen Hemminger
2018-07-31 16:30 ` [dpdk-dev] [PATCH 4/5] mlx5: no need for " Stephen Hemminger
2018-07-31 16:30 ` [dpdk-dev] [PATCH 5/5] mlx4: remove redunant " Stephen Hemminger
2018-07-31 16:48 ` [dpdk-dev] [PATCH 0/5] remove usage of register keyword in C Adrien Mazarguil
2018-07-31 18:07   ` Stephen Hemminger
2018-08-01 18:03     ` Yongseok Koh
2018-08-01 21:03       ` Stephen Hemminger
2018-08-23 13:07         ` Ferruh Yigit
2018-10-09  9:19           ` Ferruh Yigit
2019-01-31  8:02             ` Tom Barbette
2019-01-31  9:11               ` Bruce Richardson
2019-01-31  9:39                 ` Tom Barbette
2019-01-31 17:34                 ` Wiles, Keith
2018-08-01 10:18 ` Matan Azrad

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).