[dpdk-dev,v2,2/4] Driver/Mellanox: fix PMD compiling issue

Message ID 1526376227-25534-2-git-send-email-gavin.hu@arm.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Gavin Hu May 15, 2018, 9:23 a.m. UTC
  Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Signed-off-by: Sirshak Das <sirshak.das@arm.com>
Reviewed-by: Phil Yang <Phil.Yang@arm.com>
Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
---
 drivers/net/mlx5/mlx5_rxtx_vec_neon.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
  

Comments

Yongseok Koh May 15, 2018, 5:05 p.m. UTC | #1
> On May 15, 2018, at 2:23 AM, Gavin Hu <gavin.hu@arm.com> wrote:

The title should start from 'net/mlx5: ...' and please be more specific why
that pair of parentheses is needed. Add more commit messages and it'd be
better to add the error messages like you do in the other commit.

To me, it still looks unnecessary.

Thanks,
Yongseok

> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> Signed-off-by: Sirshak Das <sirshak.das@arm.com>
> Reviewed-by: Phil Yang <Phil.Yang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> ---
> drivers/net/mlx5/mlx5_rxtx_vec_neon.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
> index 2673d6b..71a5eaf 100644
> --- a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
> +++ b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
> @@ -167,8 +167,8 @@ txq_scatter_v(struct mlx5_txq_data *txq, struct rte_mbuf **pkts,
> 		vst1q_u8((void *)t_wqe, ctrl);
> 		/* Fill ESEG in the header. */
> 		vst1q_u16((void *)(t_wqe + 1),
> -			  (uint16x8_t) { 0, 0, cs_flags, rte_cpu_to_be_16(len),
> -					 0, 0, 0, 0 });
> +			  ((uint16x8_t) { 0, 0, cs_flags, rte_cpu_to_be_16(len),
> +					  0, 0, 0, 0 }));
> 		txq->wqe_ci = wqe_ci;
> 	}
> 	if (!n)
> @@ -300,10 +300,10 @@ txq_burst_v(struct mlx5_txq_data *txq, struct rte_mbuf **pkts, uint16_t pkts_n,
> 	vst1q_u8((void *)t_wqe, ctrl);
> 	/* Fill ESEG in the header. */
> 	vst1q_u8((void *)(t_wqe + 1),
> -		 (uint8x16_t) { 0, 0, 0, 0,
> -				cs_flags, 0, 0, 0,
> -				0, 0, 0, 0,
> -				0, 0, 0, 0 });
> +		 ((uint8x16_t) { 0, 0, 0, 0,
> +				 cs_flags, 0, 0, 0,
> +				 0, 0, 0, 0,
> +				 0, 0, 0, 0 }));
> #ifdef MLX5_PMD_SOFT_COUNTERS
> 	txq->stats.opackets += pkts_n;
> #endif
> -- 
> 2.1.4
>
  
Sirshak Das May 15, 2018, 5:52 p.m. UTC | #2
Hi Yongseok,

We will make the commit message more verbose.
As for the error: This is a clang compiler issue:
drivers/net/mlx5/mlx5_rxtx_vec.c:37:
/home/sirdas/code/commitc/dpdk-stable-18.02.1/drivers/net/mlx5/mlx5_rxtx_vec_neon.h:170:24: error: too many arguments provided to function-like macro invocation
                          (uint16x8_t) { 0, 0, cs_flags,
                          rte_cpu_to_be_16(len),

Thank you
Sirshak Das.

Yongseok Koh writes:

>> On May 15, 2018, at 2:23 AM, Gavin Hu <gavin.hu@arm.com> wrote:
>
> The title should start from 'net/mlx5: ...' and please be more specific why
> that pair of parentheses is needed. Add more commit messages and it'd be
> better to add the error messages like you do in the other commit.
>
> To me, it still looks unnecessary.
>
> Thanks,
> Yongseok
>
>> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
>> Signed-off-by: Sirshak Das <sirshak.das@arm.com>
>> Reviewed-by: Phil Yang <Phil.Yang@arm.com>
>> Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
>> ---
>> drivers/net/mlx5/mlx5_rxtx_vec_neon.h | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
>> index 2673d6b..71a5eaf 100644
>> --- a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
>> +++ b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
>> @@ -167,8 +167,8 @@ txq_scatter_v(struct mlx5_txq_data *txq, struct rte_mbuf **pkts,
>> 		vst1q_u8((void *)t_wqe, ctrl);
>> 		/* Fill ESEG in the header. */
>> 		vst1q_u16((void *)(t_wqe + 1),
>> -			  (uint16x8_t) { 0, 0, cs_flags, rte_cpu_to_be_16(len),
>> -					 0, 0, 0, 0 });
>> +			  ((uint16x8_t) { 0, 0, cs_flags, rte_cpu_to_be_16(len),
>> +					  0, 0, 0, 0 }));
>> 		txq->wqe_ci = wqe_ci;
>> 	}
>> 	if (!n)
>> @@ -300,10 +300,10 @@ txq_burst_v(struct mlx5_txq_data *txq, struct rte_mbuf **pkts, uint16_t pkts_n,
>> 	vst1q_u8((void *)t_wqe, ctrl);
>> 	/* Fill ESEG in the header. */
>> 	vst1q_u8((void *)(t_wqe + 1),
>> -		 (uint8x16_t) { 0, 0, 0, 0,
>> -				cs_flags, 0, 0, 0,
>> -				0, 0, 0, 0,
>> -				0, 0, 0, 0 });
>> +		 ((uint8x16_t) { 0, 0, 0, 0,
>> +				 cs_flags, 0, 0, 0,
>> +				 0, 0, 0, 0,
>> +				 0, 0, 0, 0 }));
>> #ifdef MLX5_PMD_SOFT_COUNTERS
>> 	txq->stats.opackets += pkts_n;
>> #endif
>> -- 
>> 2.1.4
>>
  
Yongseok Koh May 15, 2018, 6:38 p.m. UTC | #3
> On May 15, 2018, at 10:52 AM, Sirshak Das <sirshak.das@arm.com> wrote:
> 
> Hi Yongseok,
> 
> We will make the commit message more verbose.
> As for the error: This is a clang compiler issue:
> drivers/net/mlx5/mlx5_rxtx_vec.c:37:
> /home/sirdas/code/commitc/dpdk-stable-18.02.1/drivers/net/mlx5/mlx5_rxtx_vec_neon.h:170:24: error: too many arguments provided to function-like macro invocation
>                          (uint16x8_t) { 0, 0, cs_flags,
>                          rte_cpu_to_be_16(len),

Thanks for explanation.

Please add my acked-by tag when you submit the new version with
a) title change b) verbose commit message.

Acked-by: Yongseok Koh <yskoh@mellanox.com>
 
Thank you for contribution!
Yongseok

> Yongseok Koh writes:
> 
>>> On May 15, 2018, at 2:23 AM, Gavin Hu <gavin.hu@arm.com> wrote:
>> 
>> The title should start from 'net/mlx5: ...' and please be more specific why
>> that pair of parentheses is needed. Add more commit messages and it'd be
>> better to add the error messages like you do in the other commit.
>> 
>> To me, it still looks unnecessary.
>> 
>> Thanks,
>> Yongseok
>> 
>>> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
>>> Signed-off-by: Sirshak Das <sirshak.das@arm.com>
>>> Reviewed-by: Phil Yang <Phil.Yang@arm.com>
>>> Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
>>> ---
>>> drivers/net/mlx5/mlx5_rxtx_vec_neon.h | 12 ++++++------
>>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
>>> index 2673d6b..71a5eaf 100644
>>> --- a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
>>> +++ b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
>>> @@ -167,8 +167,8 @@ txq_scatter_v(struct mlx5_txq_data *txq, struct rte_mbuf **pkts,
>>> 		vst1q_u8((void *)t_wqe, ctrl);
>>> 		/* Fill ESEG in the header. */
>>> 		vst1q_u16((void *)(t_wqe + 1),
>>> -			  (uint16x8_t) { 0, 0, cs_flags, rte_cpu_to_be_16(len),
>>> -					 0, 0, 0, 0 });
>>> +			  ((uint16x8_t) { 0, 0, cs_flags, rte_cpu_to_be_16(len),
>>> +					  0, 0, 0, 0 }));
>>> 		txq->wqe_ci = wqe_ci;
>>> 	}
>>> 	if (!n)
>>> @@ -300,10 +300,10 @@ txq_burst_v(struct mlx5_txq_data *txq, struct rte_mbuf **pkts, uint16_t pkts_n,
>>> 	vst1q_u8((void *)t_wqe, ctrl);
>>> 	/* Fill ESEG in the header. */
>>> 	vst1q_u8((void *)(t_wqe + 1),
>>> -		 (uint8x16_t) { 0, 0, 0, 0,
>>> -				cs_flags, 0, 0, 0,
>>> -				0, 0, 0, 0,
>>> -				0, 0, 0, 0 });
>>> +		 ((uint8x16_t) { 0, 0, 0, 0,
>>> +				 cs_flags, 0, 0, 0,
>>> +				 0, 0, 0, 0,
>>> +				 0, 0, 0, 0 }));
>>> #ifdef MLX5_PMD_SOFT_COUNTERS
>>> 	txq->stats.opackets += pkts_n;
>>> #endif
>>> -- 
>>> 2.1.4
>>> 
>
  

Patch

diff --git a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
index 2673d6b..71a5eaf 100644
--- a/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
+++ b/drivers/net/mlx5/mlx5_rxtx_vec_neon.h
@@ -167,8 +167,8 @@  txq_scatter_v(struct mlx5_txq_data *txq, struct rte_mbuf **pkts,
 		vst1q_u8((void *)t_wqe, ctrl);
 		/* Fill ESEG in the header. */
 		vst1q_u16((void *)(t_wqe + 1),
-			  (uint16x8_t) { 0, 0, cs_flags, rte_cpu_to_be_16(len),
-					 0, 0, 0, 0 });
+			  ((uint16x8_t) { 0, 0, cs_flags, rte_cpu_to_be_16(len),
+					  0, 0, 0, 0 }));
 		txq->wqe_ci = wqe_ci;
 	}
 	if (!n)
@@ -300,10 +300,10 @@  txq_burst_v(struct mlx5_txq_data *txq, struct rte_mbuf **pkts, uint16_t pkts_n,
 	vst1q_u8((void *)t_wqe, ctrl);
 	/* Fill ESEG in the header. */
 	vst1q_u8((void *)(t_wqe + 1),
-		 (uint8x16_t) { 0, 0, 0, 0,
-				cs_flags, 0, 0, 0,
-				0, 0, 0, 0,
-				0, 0, 0, 0 });
+		 ((uint8x16_t) { 0, 0, 0, 0,
+				 cs_flags, 0, 0, 0,
+				 0, 0, 0, 0,
+				 0, 0, 0, 0 }));
 #ifdef MLX5_PMD_SOFT_COUNTERS
 	txq->stats.opackets += pkts_n;
 #endif