[PATCH] net/virtio: fix build error with clang

Interestingly, clang and gcc has different prototype for _mm_prefetch().
For gcc, we have

   _mm_prefetch (const void *__P, enum _mm_hint __I)

While for clang, it's

   #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a), 0, (sel)))

That how the following error comes with clang:

   error: cast from 'const void *' to 'void *' drops const qualifier
   [-Werror,-Wcast-qual]
           _mm_prefetch((const void *)rused, _MM_HINT_T0);
   /usr/lib/llvm-3.8/bin/../lib/clang/3.8.0/include/xmmintrin.h:684:58:
   note: expanded from macro '_mm_prefetch'
            #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a),
                                          0, (sel)))

What's weird is that the build was actaully Okay before. I met it while
apply Jerin's vector support for ARM patch set: he just move this peiece
of code to another file, nothing else changed.

This patch fix the issue when Jerin's patchset is applied. Thus, I think
it's still needed.

Fixes: fc3d66212fed ("virtio: add vector Rx")

Cc: Jerin Jacob <redacted>
Signed-off-by: Yuanhan Liu <redacted>
---
 drivers/net/virtio/virtio_rxtx_simple.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
index 6517aa8..56195e5 100644
--- a/drivers/net/virtio/virtio_rxtx_simple.c
+++ b/drivers/net/virtio/virtio_rxtx_simple.c

@@ -200,7 +200,7 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	sw_ring  = &vq->sw_ring[desc_idx];
 	sw_ring_end = &vq->sw_ring[vq->vq_nentries];
 
-	_mm_prefetch((const void *)rused, _MM_HINT_T0);
+	_mm_prefetch(rused, _MM_HINT_T0);
 
 	if (vq->vq_free_cnt >= RTE_VIRTIO_VPMD_RX_REARM_THRESH) {
 		virtio_rxq_rearm_vec(rxvq);

-- 
1.9.0

On Tue, Aug 23, 2016 at 03:40:58PM +0800, Yuanhan Liu wrote:

Interestingly, clang and gcc has different prototype for _mm_prefetch().
For gcc, we have

   _mm_prefetch (const void *__P, enum _mm_hint __I)

While for clang, it's

   #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a), 0, (sel)))

That how the following error comes with clang:

   error: cast from 'const void *' to 'void *' drops const qualifier
   [-Werror,-Wcast-qual]
           _mm_prefetch((const void *)rused, _MM_HINT_T0);
   /usr/lib/llvm-3.8/bin/../lib/clang/3.8.0/include/xmmintrin.h:684:58:
   note: expanded from macro '_mm_prefetch'
            #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a),
                                          0, (sel)))

What's weird is that the build was actaully Okay before. I met it while
apply Jerin's vector support for ARM patch set: he just move this peiece
of code to another file, nothing else changed.

This patch fix the issue when Jerin's patchset is applied. Thus, I think
it's still needed.

Unfortunately, it makes the icc build fail. I will submit another patch
to simply change _mm_prefetch to rte_prefetch0.

	--yliu

Interestingly, clang and gcc has different prototype for _mm_prefetch().
For gcc, we have

   _mm_prefetch (const void *__P, enum _mm_hint __I)

While for clang, it's

   #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a), 0, (sel)))

That how the following error comes with clang:

   error: cast from 'const void *' to 'void *' drops const qualifier
   [-Werror,-Wcast-qual]
           _mm_prefetch((const void *)rused, _MM_HINT_T0);
   /usr/lib/llvm-3.8/bin/../lib/clang/3.8.0/include/xmmintrin.h:684:58:
   note: expanded from macro '_mm_prefetch'
            #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a),
                                          0, (sel)))

What's weird is that the build was actaully Okay before. I met it while
apply Jerin's vector support for ARM patch set: he just move this peiece
of code to another file, nothing else changed.

This patch fix the issue when Jerin's patchset is applied. Thus, I think
it's still needed.

Fixes: fc3d66212fed ("virtio: add vector Rx")

Cc: Jerin Jacob <redacted>
Signed-off-by: Yuanhan Liu <redacted>
---
v2: replace _mm_prefetch with rte_prefetch0 to make icc happy
---
 drivers/net/virtio/virtio_rxtx_simple.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
index 6517aa8..d8dd17c 100644
--- a/drivers/net/virtio/virtio_rxtx_simple.c
+++ b/drivers/net/virtio/virtio_rxtx_simple.c

@@ -200,7 +200,7 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	sw_ring  = &vq->sw_ring[desc_idx];
 	sw_ring_end = &vq->sw_ring[vq->vq_nentries];
 
-	_mm_prefetch((const void *)rused, _MM_HINT_T0);
+	rte_prefetch0(rused);
 
 	if (vq->vq_free_cnt >= RTE_VIRTIO_VPMD_RX_REARM_THRESH) {
 		virtio_rxq_rearm_vec(rxvq);

-- 
1.9.0

On Fri, Sep 23, 2016 at 02:15:09PM +0800, Yuanhan Liu wrote:

Hi Yuanhan,

Thanks for this patch.

Interestingly, clang and gcc has different prototype for _mm_prefetch().
For gcc, we have

   _mm_prefetch (const void *__P, enum _mm_hint __I)

While for clang, it's

   #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a), 0, (sel)))

That how the following error comes with clang:

   error: cast from 'const void *' to 'void *' drops const qualifier
   [-Werror,-Wcast-qual]
           _mm_prefetch((const void *)rused, _MM_HINT_T0);
   /usr/lib/llvm-3.8/bin/../lib/clang/3.8.0/include/xmmintrin.h:684:58:
   note: expanded from macro '_mm_prefetch'
            #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a),
                                          0, (sel)))

What's weird is that the build was actaully Okay before. I met it while
apply Jerin's vector support for ARM patch set: he just move this peiece
of code to another file, nothing else changed.

This patch fix the issue when Jerin's patchset is applied. Thus, I think
it's still needed.

The info notes can be moved under the "---" marker line to make git log
clean.

This patch review is holding the virtio arm NEON support. Appreciate any help
in reviewing this patch.

http://dpdk.org/dev/patchwork/patch/14567/

On Fri, Sep 23, 2016 at 12:05:14PM +0530, Jerin Jacob wrote:

On Fri, Sep 23, 2016 at 02:15:09PM +0800, Yuanhan Liu wrote:

Hi Yuanhan,

Thanks for this patch.

quoted

Interestingly, clang and gcc has different prototype for _mm_prefetch().
For gcc, we have

   _mm_prefetch (const void *__P, enum _mm_hint __I)

While for clang, it's

   #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a), 0, (sel)))

That how the following error comes with clang:

   error: cast from 'const void *' to 'void *' drops const qualifier
   [-Werror,-Wcast-qual]
           _mm_prefetch((const void *)rused, _MM_HINT_T0);
   /usr/lib/llvm-3.8/bin/../lib/clang/3.8.0/include/xmmintrin.h:684:58:
   note: expanded from macro '_mm_prefetch'
            #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a),
                                          0, (sel)))

What's weird is that the build was actaully Okay before. I met it while
apply Jerin's vector support for ARM patch set: he just move this peiece
of code to another file, nothing else changed.

This patch fix the issue when Jerin's patchset is applied. Thus, I think
it's still needed.

The info notes can be moved under the "---" marker line to make git log
clean.

Yes, kind of. The reason I want to put it here is to stree "how weird
this issue it is" :)

Without this piece, people may be confused why this patch is necessary,
because they may simply can't reproduce this issue.

This patch review is holding the virtio arm NEON support. Appreciate any help
in reviewing this patch.

http://dpdk.org/dev/patchwork/patch/14567/

I meant to apply this series long time ago, until I found a build issue.
I tried to fix it once, as you saw. But it somehow broke the icc.

And here is my 2nd try (sorry for being a bit late though).

	--yliu

2016-09-23 14:15, Yuanhan Liu:

-	_mm_prefetch((const void *)rused, _MM_HINT_T0);
+	rte_prefetch0(rused);

There are other occurences of _mm_prefetch in other drivers.
It could deserve a patch.

On Fri, Sep 23, 2016 at 10:31:12AM +0200, Thomas Monjalon wrote:

2016-09-23 14:15, Yuanhan Liu:

quoted

-	_mm_prefetch((const void *)rused, _MM_HINT_T0);
+	rte_prefetch0(rused);

There are other occurences of _mm_prefetch in other drivers.
It could deserve a patch.

Indeed. I could make a patch to replace them all if you think that's
appropriate.

	--yliu

Interestingly, clang and gcc has different prototype for _mm_prefetch().
For gcc, we have

   _mm_prefetch (const void *__P, enum _mm_hint __I)

While for clang, it's

   #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a), 0, (sel)))

That's how the following error comes with clang:

   error: cast from 'const void *' to 'void *' drops const qualifier
   [-Werror,-Wcast-qual]
           _mm_prefetch((const void *)rused, _MM_HINT_T0);
   /usr/lib/llvm-3.8/bin/../lib/clang/3.8.0/include/xmmintrin.h:684:58:
   note: expanded from macro '_mm_prefetch'
            #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a),
                                          0, (sel)))

What's weird is that the build was actaully Okay before. I met it while
apply Jerin's vector support for ARM patch set: he just move this peiece
of code to another file, nothing else changed.

This patch fix the issue when Jerin's patchset is applied. Thus, I think
it's still needed.

Similarly, make the same change to other _mm_prefetch users, just in case
this weird issue shows up again somehow later.

Fixes: fc3d66212fed ("virtio: add vector Rx")
Fixes: c95584dc2b18 ("ixgbe: new vectorized functions for Rx/Tx")
Fixes: 9ed94e5bb04e ("i40e: add vector Rx")
Fixes: 7092be8437bd ("fm10k: add vector Rx")

Cc: Jerin Jacob <redacted>
Cc: Chen Jing D(Mark) <redacted>
Cc: Cunming Liang <redacted>
Cc: Bruce Richardson <redacted>
CC: Thomas Monjalon <redacted>
Signed-off-by: Yuanhan Liu <redacted>
---
v3: make the similar replace to other PMD
---
 drivers/net/fm10k/fm10k_rxtx_vec.c      | 2 +-
 drivers/net/i40e/i40e_rxtx_vec.c        | 2 +-
 drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c  | 2 +-
 drivers/net/virtio/virtio_rxtx_simple.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c b/drivers/net/fm10k/fm10k_rxtx_vec.c
index 9ea747e..6b78c0f 100644
--- a/drivers/net/fm10k/fm10k_rxtx_vec.c
+++ b/drivers/net/fm10k/fm10k_rxtx_vec.c

@@ -406,7 +406,7 @@ fm10k_recv_raw_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	 */
 	rxdp = rxq->hw_ring + next_dd;
 
-	_mm_prefetch((const void *)rxdp, _MM_HINT_T0);
+	rte_prefetch0(rxdp);
 
 	/* See if we need to rearm the RX queue - gives the prefetch a bit
 	 * of time to act

diff --git a/drivers/net/i40e/i40e_rxtx_vec.c b/drivers/net/i40e/i40e_rxtx_vec.c
index 51fb282..a9ca515 100644
--- a/drivers/net/i40e/i40e_rxtx_vec.c
+++ b/drivers/net/i40e/i40e_rxtx_vec.c

@@ -224,7 +224,7 @@ _recv_raw_pkts_vec(struct i40e_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 	 */
 	rxdp = rxq->rx_ring + rxq->rx_tail;
 
-	_mm_prefetch((const void *)rxdp, _MM_HINT_T0);
+	rte_prefetch0(rxdp);
 
 	/* See if we need to rearm the RX queue - gives the prefetch a bit
 	 * of time to act

diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
index 1c4fd7c..94dfbd6 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c

@@ -243,7 +243,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 	 */
 	rxdp = rxq->rx_ring + rxq->rx_tail;
 
-	_mm_prefetch((const void *)rxdp, _MM_HINT_T0);
+	rte_prefetch0(rxdp);
 
 	/* See if we need to rearm the RX queue - gives the prefetch a bit
 	 * of time to act

diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
index 6517aa8..d8dd17c 100644
--- a/drivers/net/virtio/virtio_rxtx_simple.c
+++ b/drivers/net/virtio/virtio_rxtx_simple.c

@@ -200,7 +200,7 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	sw_ring  = &vq->sw_ring[desc_idx];
 	sw_ring_end = &vq->sw_ring[vq->vq_nentries];
 
-	_mm_prefetch((const void *)rused, _MM_HINT_T0);
+	rte_prefetch0(rused);
 
 	if (vq->vq_free_cnt >= RTE_VIRTIO_VPMD_RX_REARM_THRESH) {
 		virtio_rxq_rearm_vec(rxvq);

-- 
1.9.0

Can any PMD guys review it? It blocks a virtio patchset apply...

Thanks.

	--yliu

On Mon, Sep 26, 2016 at 12:29:13PM +0800, Yuanhan Liu wrote:

Hi, 

-----Original Message-----
From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
Sent: Tuesday, September 27, 2016 5:20 AM
To: dev@dpdk.org
Cc: Jerin Jacob <redacted>; Chen, Jing D
[off-list ref]; Liang, Cunming [off-list ref];
Richardson, Bruce [off-list ref]; Thomas Monjalon
[off-list ref]
Subject: Re: [PATCH v3] net: fix build error with clang

Can any PMD guys review it? It blocks a virtio patchset apply...

Thanks.

	--yliu

On Mon, Sep 26, 2016 at 12:29:13PM +0800, Yuanhan Liu wrote:

quoted

Interestingly, clang and gcc has different prototype for _mm_prefetch().
For gcc, we have

   _mm_prefetch (const void *__P, enum _mm_hint __I)

While for clang, it's

   #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a), 0,
(sel)))

That's how the following error comes with clang:

   error: cast from 'const void *' to 'void *' drops const qualifier
   [-Werror,-Wcast-qual]
           _mm_prefetch((const void *)rused, _MM_HINT_T0);
   /usr/lib/llvm-3.8/bin/../lib/clang/3.8.0/include/xmmintrin.h:684:58:
   note: expanded from macro '_mm_prefetch'
            #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a),
                                          0, (sel)))

What's weird is that the build was actaully Okay before. I met it
while apply Jerin's vector support for ARM patch set: he just move
this peiece of code to another file, nothing else changed.

This patch fix the issue when Jerin's patchset is applied. Thus, I
think it's still needed.

Similarly, make the same change to other _mm_prefetch users, just in
case this weird issue shows up again somehow later.

Fixes: fc3d66212fed ("virtio: add vector Rx")
Fixes: c95584dc2b18 ("ixgbe: new vectorized functions for Rx/Tx")
Fixes: 9ed94e5bb04e ("i40e: add vector Rx")
Fixes: 7092be8437bd ("fm10k: add vector Rx")

Cc: Jerin Jacob <redacted>
Cc: Chen Jing D(Mark) <redacted>
Cc: Cunming Liang <redacted>
Cc: Bruce Richardson <redacted>
CC: Thomas Monjalon <redacted>
Signed-off-by: Yuanhan Liu <redacted>

Acked-by: Jing Chen <redacted>

On Tue, Sep 27, 2016 at 05:24:35PM +0000, Chen, Jing D wrote:

quoted

On Mon, Sep 26, 2016 at 12:29:13PM +0800, Yuanhan Liu wrote:

quoted

Interestingly, clang and gcc has different prototype for _mm_prefetch().
For gcc, we have

   _mm_prefetch (const void *__P, enum _mm_hint __I)

While for clang, it's

   #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a), 0,
(sel)))

That's how the following error comes with clang:

   error: cast from 'const void *' to 'void *' drops const qualifier
   [-Werror,-Wcast-qual]
           _mm_prefetch((const void *)rused, _MM_HINT_T0);
   /usr/lib/llvm-3.8/bin/../lib/clang/3.8.0/include/xmmintrin.h:684:58:
   note: expanded from macro '_mm_prefetch'
            #define _mm_prefetch(a, sel) (__builtin_prefetch((void *)(a),
                                          0, (sel)))

What's weird is that the build was actaully Okay before. I met it
while apply Jerin's vector support for ARM patch set: he just move
this peiece of code to another file, nothing else changed.

This patch fix the issue when Jerin's patchset is applied. Thus, I
think it's still needed.

Similarly, make the same change to other _mm_prefetch users, just in
case this weird issue shows up again somehow later.

...

quoted

quoted

Signed-off-by: Yuanhan Liu <redacted>

Acked-by: Jing Chen <redacted>

Mark, thank you!

Applied to dpdk-next-virtio.

	--yliu