DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] eal: add new prefetch0_write variant
@ 2020-09-11  9:19 Harry van Haaren
  2020-09-13 20:11 ` Pavan Nikhilesh Bhagavatula
  2020-09-14 15:10 ` [dpdk-dev] [PATCH v2] eal: add new prefetch write variants Harry van Haaren
  0 siblings, 2 replies; 15+ messages in thread
From: Harry van Haaren @ 2020-09-11  9:19 UTC (permalink / raw)
  To: dev; +Cc: Harry van Haaren

This commit adds a new rte_prefetch0_write() variant, suggests to the
compiler to use a prefetch instruction with intention to write. As a
compiler builtin, the compiler can choose based on compilation target
what the best implementation for this instruction is.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

The integer constants passed to the builtin are not available as
a #define value, and doing #defines just for this write variant
does not seems a nice solution to me... particularly for those using
IDEs where any #define value is auto-hinted for code-completion.

---
 lib/librte_eal/include/generic/rte_prefetch.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/lib/librte_eal/include/generic/rte_prefetch.h b/lib/librte_eal/include/generic/rte_prefetch.h
index 6e47bdfbad..44e2e9abfc 100644
--- a/lib/librte_eal/include/generic/rte_prefetch.h
+++ b/lib/librte_eal/include/generic/rte_prefetch.h
@@ -51,4 +51,20 @@ static inline void rte_prefetch2(const volatile void *p);
  */
 static inline void rte_prefetch_non_temporal(const volatile void *p);
 
+/**
+ * Prefetch a cache line into all cache levels, with intention to write. This
+ * prefetch variant hints to the CPU that the program is expecting to write to
+ * the cache line being prefetched.
+ *
+ * @param p Address to prefetch
+ */
+static inline void rte_prefetch0_write(const void *p)
+{
+	/* 1 indicates intention to write, 3 sets target cache level to L1. See
+	 * GCC docs where these integer constants are described in more detail:
+	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
+	 */
+	__builtin_prefetch(p, 1, 3);
+}
+
 #endif /* _RTE_PREFETCH_H_ */
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH] eal: add new prefetch0_write variant
  2020-09-11  9:19 [dpdk-dev] [PATCH] eal: add new prefetch0_write variant Harry van Haaren
@ 2020-09-13 20:11 ` Pavan Nikhilesh Bhagavatula
  2020-09-14  8:12   ` Van Haaren, Harry
  2020-09-14 15:10 ` [dpdk-dev] [PATCH v2] eal: add new prefetch write variants Harry van Haaren
  1 sibling, 1 reply; 15+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2020-09-13 20:11 UTC (permalink / raw)
  To: Harry van Haaren, dev

>This commit adds a new rte_prefetch0_write() variant, suggests to the
>compiler to use a prefetch instruction with intention to write. As a
>compiler builtin, the compiler can choose based on compilation target
>what the best implementation for this instruction is.	

Why not have the other variants too i.e. l2/l3/temporal store prefetches too?


>
>Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
>
>---
>
>The integer constants passed to the builtin are not available as
>a #define value, and doing #defines just for this write variant
>does not seems a nice solution to me... particularly for those using
>IDEs where any #define value is auto-hinted for code-completion.
>
>---
> lib/librte_eal/include/generic/rte_prefetch.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
>diff --git a/lib/librte_eal/include/generic/rte_prefetch.h
>b/lib/librte_eal/include/generic/rte_prefetch.h
>index 6e47bdfbad..44e2e9abfc 100644
>--- a/lib/librte_eal/include/generic/rte_prefetch.h
>+++ b/lib/librte_eal/include/generic/rte_prefetch.h
>@@ -51,4 +51,20 @@ static inline void rte_prefetch2(const volatile
>void *p);
>  */
> static inline void rte_prefetch_non_temporal(const volatile void *p);
>
>+/**
>+ * Prefetch a cache line into all cache levels, with intention to write.
>This
>+ * prefetch variant hints to the CPU that the program is expecting to
>write to
>+ * the cache line being prefetched.
>+ *
>+ * @param p Address to prefetch
>+ */
>+static inline void rte_prefetch0_write(const void *p)
>+{
>+	/* 1 indicates intention to write, 3 sets target cache level to L1.
>See
>+	 * GCC docs where these integer constants are described in
>more detail:
>+	 *  https://urldefense.proofpoint.com/v2/url?u=https-
>3A__gcc.gnu.org_onlinedocs_gcc_Other-
>2DBuiltins.html&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=1cjuAHrG
>h745jHNmj2fD85sUMIJ2IPIDsIJzo6FN6Z0&m=OMPc_t21vsWlzVl0UjdGB
>hTTv1Gngqfk8vth9UQtUSA&s=51jgfiV2UC5B3xxBE-
>4gRAte3lCZP3v370U7Fpn6LaE&e=
>+	 */
>+	__builtin_prefetch(p, 1, 3);
>+}
>+
> #endif /* _RTE_PREFETCH_H_ */
>--
>2.17.1


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

* Re: [dpdk-dev] [PATCH] eal: add new prefetch0_write variant
  2020-09-13 20:11 ` Pavan Nikhilesh Bhagavatula
@ 2020-09-14  8:12   ` Van Haaren, Harry
  2020-09-14 10:39     ` Pavan Nikhilesh Bhagavatula
  0 siblings, 1 reply; 15+ messages in thread
From: Van Haaren, Harry @ 2020-09-14  8:12 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, dev

> -----Original Message-----
> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Sent: Sunday, September 13, 2020 9:11 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] eal: add new prefetch0_write variant
> 
> >This commit adds a new rte_prefetch0_write() variant, suggests to the
> >compiler to use a prefetch instruction with intention to write. As a
> >compiler builtin, the compiler can choose based on compilation target
> >what the best implementation for this instruction is.
> 
> Why not have the other variants too i.e. l2/l3/temporal store prefetches too?

Hi Pavan,

Are there architectures that actually implement those? Usually for a WB mem store to complete,
the data must be present in L1 cache (on x86 at least), and that's what the patch below with write0 achieves.

I'm against adding all the variants "just in case", it leads to API bloat, and increases
cognitive load on the programmer. My expectation is that in 99% of usage the prefetch
write instruction should target L1.

Cheers, -Harry

> >Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> >
> >---
> >
> >The integer constants passed to the builtin are not available as
> >a #define value, and doing #defines just for this write variant
> >does not seems a nice solution to me... particularly for those using
> >IDEs where any #define value is auto-hinted for code-completion.
> >
> >---
> > lib/librte_eal/include/generic/rte_prefetch.h | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)

<snip patch contents>


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

* Re: [dpdk-dev] [PATCH] eal: add new prefetch0_write variant
  2020-09-14  8:12   ` Van Haaren, Harry
@ 2020-09-14 10:39     ` Pavan Nikhilesh Bhagavatula
  2020-09-14 15:10       ` Van Haaren, Harry
  0 siblings, 1 reply; 15+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2020-09-14 10:39 UTC (permalink / raw)
  To: Van Haaren, Harry, dev

>> >This commit adds a new rte_prefetch0_write() variant, suggests to
>the
>> >compiler to use a prefetch instruction with intention to write. As a
>> >compiler builtin, the compiler can choose based on compilation
>target
>> >what the best implementation for this instruction is.
>>
>> Why not have the other variants too i.e. l2/l3/temporal store
>prefetches too?
>
>Hi Pavan,
>
Hi Harry,
(LTNS)

>Are there architectures that actually implement those? Usually for a WB
>mem store to complete,
>the data must be present in L1 cache (on x86 at least), and that's what
>the patch below with write0 achieves.

ARM64 does supports all modes of store prefetch
"
<type> is one of:
PLD Prefetch for load, encoded in the "Rt<4:3>" field as 0b00.
PLI Preload instructions, encoded in the "Rt<4:3>" field as 0b01.
PST Prefetch for store, encoded in the "Rt<4:3>" field as 0b10.
<target> is one of:
L1 Level 1 cache, encoded in the "Rt<2:1>" field as 0b00.
L2 Level 2 cache, encoded in the "Rt<2:1>" field as 0b01.
L3 Level 3 cache, encoded in the "Rt<2:1>" field as 0b10.
<policy> is one of:
KEEP Retained or temporal prefetch, allocated in the cache normally. Encoded in the "Rt<0>"
field as 0.
STRM Streaming or non-temporal prefetch, for data that is used only once. Encoded in the
"Rt<0>" field as 1.
For more information on these prefetch
"

>
>I'm against adding all the variants "just in case", it leads to API bloat,
>and increases
>cognitive load on the programmer. My expectation is that in 99% of
>usage the prefetch
>write instruction should target L1.
>

There is a use case when cache mode is write through and application is 
pipelining work across cores sharing same L2 cluster.

>Cheers, -Harry

Regards,
Pavan.

>
>> >Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
>> >
>> >---
>> >
>> >The integer constants passed to the builtin are not available as
>> >a #define value, and doing #defines just for this write variant
>> >does not seems a nice solution to me... particularly for those using
>> >IDEs where any #define value is auto-hinted for code-completion.
>> >
>> >---
>> > lib/librte_eal/include/generic/rte_prefetch.h | 16
>++++++++++++++++
>> > 1 file changed, 16 insertions(+)
>
><snip patch contents>


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

* Re: [dpdk-dev] [PATCH] eal: add new prefetch0_write variant
  2020-09-14 10:39     ` Pavan Nikhilesh Bhagavatula
@ 2020-09-14 15:10       ` Van Haaren, Harry
  0 siblings, 0 replies; 15+ messages in thread
From: Van Haaren, Harry @ 2020-09-14 15:10 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, dev

> -----Original Message-----
> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Sent: Monday, September 14, 2020 11:39 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] eal: add new prefetch0_write variant
> 
> >> >This commit adds a new rte_prefetch0_write() variant, suggests to
> >the
> >> >compiler to use a prefetch instruction with intention to write. As a
> >> >compiler builtin, the compiler can choose based on compilation
> >target
> >> >what the best implementation for this instruction is.
> >>
> >> Why not have the other variants too i.e. l2/l3/temporal store
> >prefetches too?
> >
> >Hi Pavan,
> >
> Hi Harry,
> (LTNS)
> 
> >Are there architectures that actually implement those? Usually for a WB
> >mem store to complete,
> >the data must be present in L1 cache (on x86 at least), and that's what
> >the patch below with write0 achieves.
> 
> ARM64 does supports all modes of store prefetch
> "
> <type> is one of:
> PLD Prefetch for load, encoded in the "Rt<4:3>" field as 0b00.
> PLI Preload instructions, encoded in the "Rt<4:3>" field as 0b01.
> PST Prefetch for store, encoded in the "Rt<4:3>" field as 0b10.
> <target> is one of:
> L1 Level 1 cache, encoded in the "Rt<2:1>" field as 0b00.
> L2 Level 2 cache, encoded in the "Rt<2:1>" field as 0b01.
> L3 Level 3 cache, encoded in the "Rt<2:1>" field as 0b10.
> <policy> is one of:
> KEEP Retained or temporal prefetch, allocated in the cache normally. Encoded in
> the "Rt<0>"
> field as 0.
> STRM Streaming or non-temporal prefetch, for data that is used only once. Encoded
> in the
> "Rt<0>" field as 1.
> For more information on these prefetch
> "
> 
> >
> >I'm against adding all the variants "just in case", it leads to API bloat,
> >and increases
> >cognitive load on the programmer. My expectation is that in 99% of
> >usage the prefetch
> >write instruction should target L1.
> >
> 
> There is a use case when cache mode is write through and application is
> pipelining work across cores sharing same L2 cluster.

OK - v2 sent: http://patches.dpdk.org/patch/77632/

APIs matching the existing prefetch APIs:
rte_prefetch0_write() L1 and all below
rte_prefetch1_write() L2 and all below
rte_prefetch2_write() L3

Cheers, -Harry


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

* [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
  2020-09-11  9:19 [dpdk-dev] [PATCH] eal: add new prefetch0_write variant Harry van Haaren
  2020-09-13 20:11 ` Pavan Nikhilesh Bhagavatula
@ 2020-09-14 15:10 ` Harry van Haaren
  2020-10-08  7:42   ` David Marchand
                     ` (4 more replies)
  1 sibling, 5 replies; 15+ messages in thread
From: Harry van Haaren @ 2020-09-14 15:10 UTC (permalink / raw)
  To: dev; +Cc: pbhagavatula, Harry van Haaren

This commit adds a new rte_prefetch0_write() variants, suggesting to the
compiler to use a prefetch instruction with intention to write. As a
compiler builtin, the compiler can choose based on compilation target
what the best implementation for this instruction is.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

v2:
- Add L1, L2, and L3 variants as ARM64 uarch supports them (Pavan)

The integer constants passed to the builtin are not available as
a #define value, and doing #defines just for this write variant
does not seems a nice solution to me... particularly for those using
IDEs where any #define value is auto-hinted for code-completion.
---
 lib/librte_eal/include/generic/rte_prefetch.h | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/lib/librte_eal/include/generic/rte_prefetch.h b/lib/librte_eal/include/generic/rte_prefetch.h
index 6e47bdfbad..3dfca77a74 100644
--- a/lib/librte_eal/include/generic/rte_prefetch.h
+++ b/lib/librte_eal/include/generic/rte_prefetch.h
@@ -51,4 +51,53 @@ static inline void rte_prefetch2(const volatile void *p);
  */
 static inline void rte_prefetch_non_temporal(const volatile void *p);
 
+/**
+ * Prefetch a cache line into all cache levels, with intention to write. This
+ * prefetch variant hints to the CPU that the program is expecting to write to
+ * the cache line being prefetched.
+ *
+ * @param p Address to prefetch
+ */
+static inline void rte_prefetch0_write(const void *p)
+{
+	/* 1 indicates intention to write, 3 sets target cache level to L1. See
+	 * GCC docs where these integer constants are described in more detail:
+	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
+	 */
+	__builtin_prefetch(p, 1, 3);
+}
+
+/**
+ * Prefetch a cache line into all cache levels, except the 0th, with intention
+ * to write. This prefetch variant hints to the CPU that the program is
+ * expecting to write to the cache line being prefetched.
+ *
+ * @param p Address to prefetch
+ */
+static inline void rte_prefetch1_write(const void *p)
+{
+	/* 1 indicates intention to write, 2 sets target cache level to L2. See
+	 * GCC docs where these integer constants are described in more detail:
+	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
+	 */
+	__builtin_prefetch(p, 1, 2);
+}
+
+/**
+ * Prefetch a cache line into all cache levels, except the 0th and 1st, with
+ * intention to write. This prefetch variant hints to the CPU that the program
+ * is expecting to write to the cache line being prefetched.
+ *
+ * @param p Address to prefetch
+ */
+static inline void rte_prefetch2_write(const void *p)
+{
+	/* 1 indicates intention to write, 1 sets target cache level to L3. See
+	 * GCC docs where these integer constants are described in more detail:
+	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
+	 */
+	__builtin_prefetch(p, 1, 1);
+}
+
+
 #endif /* _RTE_PREFETCH_H_ */
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
  2020-09-14 15:10 ` [dpdk-dev] [PATCH v2] eal: add new prefetch write variants Harry van Haaren
@ 2020-10-08  7:42   ` David Marchand
  2020-10-08  8:34     ` Van Haaren, Harry
  2020-10-08  8:54   ` Jerin Jacob
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: David Marchand @ 2020-10-08  7:42 UTC (permalink / raw)
  To: Harry van Haaren, Bruce Richardson, Ananyev, Konstantin,
	Jerin Jacob Kollanukkaran, Ruifeng Wang (Arm Technology China),
	Jan Viktorin, David Christensen, Honnappa Nagarahalli
  Cc: dev, Pavan Nikhilesh

On Mon, Sep 14, 2020 at 5:09 PM Harry van Haaren
<harry.van.haaren@intel.com> wrote:
>
> This commit adds a new rte_prefetch0_write() variants, suggesting to the
> compiler to use a prefetch instruction with intention to write. As a
> compiler builtin, the compiler can choose based on compilation target
> what the best implementation for this instruction is.
>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

No review, copying arch maintainers.


-- 
David Marchand


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

* Re: [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
  2020-10-08  7:42   ` David Marchand
@ 2020-10-08  8:34     ` Van Haaren, Harry
  2020-10-08  8:39       ` Van Haaren, Harry
  0 siblings, 1 reply; 15+ messages in thread
From: Van Haaren, Harry @ 2020-10-08  8:34 UTC (permalink / raw)
  To: David Marchand, Richardson, Bruce, Ananyev, Konstantin,
	Jerin Jacob Kollanukkaran, Ruifeng Wang (Arm Technology China),
	Jan Viktorin, David Christensen, Honnappa Nagarahalli
  Cc: dev, Pavan Nikhilesh

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, October 8, 2020 8:43 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Ruifeng Wang (Arm Technology China) <ruifeng.wang@arm.com>; Jan Viktorin
> <viktorin@rehivetech.com>; David Christensen <drc@linux.vnet.ibm.com>;
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: dev <dev@dpdk.org>; Pavan Nikhilesh <pbhagavatula@marvell.com>
> Subject: Re: [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
> 
> On Mon, Sep 14, 2020 at 5:09 PM Harry van Haaren
> <harry.van.haaren@intel.com> wrote:
> >
> > This commit adds a new rte_prefetch0_write() variants, suggesting to the
> > compiler to use a prefetch instruction with intention to write. As a
> > compiler builtin, the compiler can choose based on compilation target
> > what the best implementation for this instruction is.
> >
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> No review, copying arch maintainers.
> 
> --
> David Marchand

Hey All,

Actually Pavan provided some feedback to add prefetch1 and prefetch2 variants:
https://mails.dpdk.org/archives/dev/2020-September/thread.html#180501

I can push a v3 with those changes later today, will CC folks here.

Thanks, -Harry

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

* Re: [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
  2020-10-08  8:34     ` Van Haaren, Harry
@ 2020-10-08  8:39       ` Van Haaren, Harry
  0 siblings, 0 replies; 15+ messages in thread
From: Van Haaren, Harry @ 2020-10-08  8:39 UTC (permalink / raw)
  To: David Marchand, Richardson, Bruce, Ananyev, Konstantin,
	Jerin Jacob Kollanukkaran, Ruifeng Wang (Arm Technology China),
	Jan Viktorin, David Christensen, Honnappa Nagarahalli
  Cc: dev, Pavan Nikhilesh

> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Thursday, October 8, 2020 9:34 AM
> To: David Marchand <david.marchand@redhat.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Ruifeng Wang (Arm Technology China) <ruifeng.wang@arm.com>; Jan Viktorin
> <viktorin@rehivetech.com>; David Christensen <drc@linux.vnet.ibm.com>;
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: dev <dev@dpdk.org>; Pavan Nikhilesh <pbhagavatula@marvell.com>
> Subject: RE: [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
> 
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Thursday, October 8, 2020 8:43 AM
> > To: Van Haaren, Harry <harry.van.haaren@intel.com>; Richardson, Bruce
> > <bruce.richardson@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>;
> > Ruifeng Wang (Arm Technology China) <ruifeng.wang@arm.com>; Jan Viktorin
> > <viktorin@rehivetech.com>; David Christensen <drc@linux.vnet.ibm.com>;
> > Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > Cc: dev <dev@dpdk.org>; Pavan Nikhilesh <pbhagavatula@marvell.com>
> > Subject: Re: [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
> >
> > On Mon, Sep 14, 2020 at 5:09 PM Harry van Haaren
> > <harry.van.haaren@intel.com> wrote:
> > >
> > > This commit adds a new rte_prefetch0_write() variants, suggesting to the
> > > compiler to use a prefetch instruction with intention to write. As a
> > > compiler builtin, the compiler can choose based on compilation target
> > > what the best implementation for this instruction is.
> > >
> > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> >
> > No review, copying arch maintainers.
> >
> > --
> > David Marchand
> 
> Hey All,
> 
> Actually Pavan provided some feedback to add prefetch1 and prefetch2 variants:
> https://mails.dpdk.org/archives/dev/2020-September/thread.html#180501
> 
> I can push a v3 with those changes later today, will CC folks here.
> 
> Thanks, -Harry

... apologies - David you're right - the v2 on list already has those changes: http://patches.dpdk.org/patch/77632/ reviews welcome!


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

* Re: [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
  2020-09-14 15:10 ` [dpdk-dev] [PATCH v2] eal: add new prefetch write variants Harry van Haaren
  2020-10-08  7:42   ` David Marchand
@ 2020-10-08  8:54   ` Jerin Jacob
  2020-10-10 10:21   ` Ruifeng Wang
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Jerin Jacob @ 2020-10-08  8:54 UTC (permalink / raw)
  To: Harry van Haaren; +Cc: dpdk-dev, Pavan Nikhilesh

On Mon, Sep 14, 2020 at 8:39 PM Harry van Haaren
<harry.van.haaren@intel.com> wrote:
>
> This commit adds a new rte_prefetch0_write() variants, suggesting to the
> compiler to use a prefetch instruction with intention to write. As a
> compiler builtin, the compiler can choose based on compilation target
> what the best implementation for this instruction is.
>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
>
> ---
>
> v2:
> - Add L1, L2, and L3 variants as ARM64 uarch supports them (Pavan)
>
> The integer constants passed to the builtin are not available as
> a #define value, and doing #defines just for this write variant
> does not seems a nice solution to me... particularly for those using
> IDEs where any #define value is auto-hinted for code-completion.
> ---
>  lib/librte_eal/include/generic/rte_prefetch.h | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
>
> diff --git a/lib/librte_eal/include/generic/rte_prefetch.h b/lib/librte_eal/include/generic/rte_prefetch.h
> index 6e47bdfbad..3dfca77a74 100644
> --- a/lib/librte_eal/include/generic/rte_prefetch.h
> +++ b/lib/librte_eal/include/generic/rte_prefetch.h
> @@ -51,4 +51,53 @@ static inline void rte_prefetch2(const volatile void *p);
>   */
>  static inline void rte_prefetch_non_temporal(const volatile void *p);
>
> +/**
> + * Prefetch a cache line into all cache levels, with intention to write. This
> + * prefetch variant hints to the CPU that the program is expecting to write to
> + * the cache line being prefetched.
> + *
> + * @param p Address to prefetch
> + */
> +static inline void rte_prefetch0_write(const void *p)

Typically DPDK coding standards is to have

static inline void
rte_prefetch0_write(const void *p)

vs
static inline void rte_prefetch0_write(const void *p)

Either way:

Reviewed-by: Jerin Jacob <jerinj@marvell.com>


> +{
> +       /* 1 indicates intention to write, 3 sets target cache level to L1. See
> +        * GCC docs where these integer constants are described in more detail:
> +        *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
> +        */
> +       __builtin_prefetch(p, 1, 3);
> +}
> +
> +/**
> + * Prefetch a cache line into all cache levels, except the 0th, with intention
> + * to write. This prefetch variant hints to the CPU that the program is
> + * expecting to write to the cache line being prefetched.
> + *
> + * @param p Address to prefetch
> + */
> +static inline void rte_prefetch1_write(const void *p)
> +{
> +       /* 1 indicates intention to write, 2 sets target cache level to L2. See
> +        * GCC docs where these integer constants are described in more detail:
> +        *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
> +        */
> +       __builtin_prefetch(p, 1, 2);
> +}
> +
> +/**
> + * Prefetch a cache line into all cache levels, except the 0th and 1st, with
> + * intention to write. This prefetch variant hints to the CPU that the program
> + * is expecting to write to the cache line being prefetched.
> + *
> + * @param p Address to prefetch
> + */
> +static inline void rte_prefetch2_write(const void *p)
> +{
> +       /* 1 indicates intention to write, 1 sets target cache level to L3. See
> +        * GCC docs where these integer constants are described in more detail:
> +        *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
> +        */
> +       __builtin_prefetch(p, 1, 1);
> +}
> +
> +
>  #endif /* _RTE_PREFETCH_H_ */
> --
> 2.17.1
>

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

* Re: [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
  2020-09-14 15:10 ` [dpdk-dev] [PATCH v2] eal: add new prefetch write variants Harry van Haaren
  2020-10-08  7:42   ` David Marchand
  2020-10-08  8:54   ` Jerin Jacob
@ 2020-10-10 10:21   ` Ruifeng Wang
  2020-10-15  8:18   ` David Marchand
  2020-10-15 10:32   ` [dpdk-dev] [PATCH v3] " Harry van Haaren
  4 siblings, 0 replies; 15+ messages in thread
From: Ruifeng Wang @ 2020-10-10 10:21 UTC (permalink / raw)
  To: Harry van Haaren, dev; +Cc: pbhagavatula, nd


> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Harry van Haaren
> Sent: Monday, September 14, 2020 11:10 PM
> To: dev@dpdk.org
> Cc: pbhagavatula@marvell.com; Harry van Haaren
> <harry.van.haaren@intel.com>
> Subject: [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
> 
> This commit adds a new rte_prefetch0_write() variants, suggesting to the
> compiler to use a prefetch instruction with intention to write. As a compiler
> builtin, the compiler can choose based on compilation target what the best
> implementation for this instruction is.
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> ---
> 
> v2:
> - Add L1, L2, and L3 variants as ARM64 uarch supports them (Pavan)
> 
> The integer constants passed to the builtin are not available as a #define
> value, and doing #defines just for this write variant does not seems a nice
> solution to me... particularly for those using IDEs where any #define value is
> auto-hinted for code-completion.
> ---
>  lib/librte_eal/include/generic/rte_prefetch.h | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/lib/librte_eal/include/generic/rte_prefetch.h
> b/lib/librte_eal/include/generic/rte_prefetch.h
> index 6e47bdfbad..3dfca77a74 100644
> --- a/lib/librte_eal/include/generic/rte_prefetch.h
> +++ b/lib/librte_eal/include/generic/rte_prefetch.h
> @@ -51,4 +51,53 @@ static inline void rte_prefetch2(const volatile void *p);
>   */
>  static inline void rte_prefetch_non_temporal(const volatile void *p);
> 
> +/**
> + * Prefetch a cache line into all cache levels, with intention to
> +write. This
> + * prefetch variant hints to the CPU that the program is expecting to
> +write to
> + * the cache line being prefetched.
> + *
> + * @param p Address to prefetch
> + */
> +static inline void rte_prefetch0_write(const void *p) {
> +	/* 1 indicates intention to write, 3 sets target cache level to L1. See
> +	 * GCC docs where these integer constants are described in more
> detail:
> +	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
> +	 */
> +	__builtin_prefetch(p, 1, 3);
> +}
> +
> +/**
> + * Prefetch a cache line into all cache levels, except the 0th, with
> +intention
> + * to write. This prefetch variant hints to the CPU that the program is
> + * expecting to write to the cache line being prefetched.
> + *
> + * @param p Address to prefetch
> + */
> +static inline void rte_prefetch1_write(const void *p) {
> +	/* 1 indicates intention to write, 2 sets target cache level to L2. See
> +	 * GCC docs where these integer constants are described in more
> detail:
> +	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
> +	 */
> +	__builtin_prefetch(p, 1, 2);
> +}
> +
> +/**
> + * Prefetch a cache line into all cache levels, except the 0th and 1st,
> +with
> + * intention to write. This prefetch variant hints to the CPU that the
> +program
> + * is expecting to write to the cache line being prefetched.
> + *
> + * @param p Address to prefetch
> + */
> +static inline void rte_prefetch2_write(const void *p) {
> +	/* 1 indicates intention to write, 1 sets target cache level to L3. See
> +	 * GCC docs where these integer constants are described in more
> detail:
> +	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
> +	 */
> +	__builtin_prefetch(p, 1, 1);
> +}
> +
> +
>  #endif /* _RTE_PREFETCH_H_ */
> --
> 2.17.1

Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>

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

* Re: [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
  2020-09-14 15:10 ` [dpdk-dev] [PATCH v2] eal: add new prefetch write variants Harry van Haaren
                     ` (2 preceding siblings ...)
  2020-10-10 10:21   ` Ruifeng Wang
@ 2020-10-15  8:18   ` David Marchand
  2020-10-15  8:44     ` Van Haaren, Harry
  2020-10-15 10:32   ` [dpdk-dev] [PATCH v3] " Harry van Haaren
  4 siblings, 1 reply; 15+ messages in thread
From: David Marchand @ 2020-10-15  8:18 UTC (permalink / raw)
  To: Harry van Haaren
  Cc: dev, Pavan Nikhilesh, Jerin Jacob Kollanukkaran,
	Ruifeng Wang (Arm Technology China)

Hello Harry,

On Mon, Sep 14, 2020 at 5:09 PM Harry van Haaren
<harry.van.haaren@intel.com> wrote:
>
> This commit adds a new rte_prefetch0_write() variants, suggesting to the
> compiler to use a prefetch instruction with intention to write. As a
> compiler builtin, the compiler can choose based on compilation target
> what the best implementation for this instruction is.
>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

There was a small comment from Jerin.

Those are new functions and should be marked as experimental.
Please add a call to those in test_prefetch.c.
Please update the release notes.

Thanks.

-- 
David Marchand


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

* Re: [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
  2020-10-15  8:18   ` David Marchand
@ 2020-10-15  8:44     ` Van Haaren, Harry
  0 siblings, 0 replies; 15+ messages in thread
From: Van Haaren, Harry @ 2020-10-15  8:44 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, Pavan Nikhilesh, Jerin Jacob Kollanukkaran,
	Ruifeng Wang (Arm Technology China)

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, October 15, 2020 9:18 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev <dev@dpdk.org>; Pavan Nikhilesh <pbhagavatula@marvell.com>; Jerin
> Jacob Kollanukkaran <jerinj@marvell.com>; Ruifeng Wang (Arm Technology
> China) <ruifeng.wang@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v2] eal: add new prefetch write variants
> 
> Hello Harry,
> 
> On Mon, Sep 14, 2020 at 5:09 PM Harry van Haaren
> <harry.van.haaren@intel.com> wrote:
> >
> > This commit adds a new rte_prefetch0_write() variants, suggesting to the
> > compiler to use a prefetch instruction with intention to write. As a
> > compiler builtin, the compiler can choose based on compilation target
> > what the best implementation for this instruction is.
> >
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> There was a small comment from Jerin.
> 
> Those are new functions and should be marked as experimental.
> Please add a call to those in test_prefetch.c.
> Please update the release notes.

Will do - thanks!

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

* [dpdk-dev] [PATCH v3] eal: add new prefetch write variants
  2020-09-14 15:10 ` [dpdk-dev] [PATCH v2] eal: add new prefetch write variants Harry van Haaren
                     ` (3 preceding siblings ...)
  2020-10-15  8:18   ` David Marchand
@ 2020-10-15 10:32   ` Harry van Haaren
  2020-10-15 20:27     ` David Marchand
  4 siblings, 1 reply; 15+ messages in thread
From: Harry van Haaren @ 2020-10-15 10:32 UTC (permalink / raw)
  To: dev; +Cc: ruifeng.wang, david.marchand, jerinj, pbhagavatula, Harry van Haaren

This commit adds new rte_prefetchX_write() variants, that suggest to the
compiler to use a prefetch instruction with intention to write. As a
compiler builtin, the compiler can choose based on compilation target
what the best implementation for this instruction is.

Three versions are provided, targeting the different levels of cache.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
Reviewed-by: Jerin Jacob <jerinj@marvell.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>

---

v3:
- Add reviewed by tags from Jerin and Ruifeng, thanks!
- Add __rte_experimental as they are new functions (David)
  This required adding the rte_compat.h include.
- Rework return type to new line (Jerin)
- Add calls in test_prefetch.c to new functions (David)
- Add item to release notes (David)

v2:
- Add L1, L2, and L3 variants as ARM64 uarch supports them (Pavan)

The integer constants passed to the builtin are not available as
a #define value, and doing #defines just for this write variant
does not seems a nice solution to me... particularly for those using
IDEs where any #define value is auto-hinted for code-completion.
---
 app/test/test_prefetch.c                      |  4 ++
 doc/guides/rel_notes/release_20_11.rst        |  6 ++
 lib/librte_eal/include/generic/rte_prefetch.h | 57 +++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/app/test/test_prefetch.c b/app/test/test_prefetch.c
index 41f219af78..32e08f8afe 100644
--- a/app/test/test_prefetch.c
+++ b/app/test/test_prefetch.c
@@ -26,6 +26,10 @@ test_prefetch(void)
 	rte_prefetch1(&a);
 	rte_prefetch2(&a);
 
+	rte_prefetch0_write(&a);
+	rte_prefetch1_write(&a);
+	rte_prefetch2_write(&a);
+
 	return 0;
 }
 
diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
index 0925123e9c..8b51ef0dbc 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -62,6 +62,12 @@ New Features
   The functions are provided as a generic stubs and
   x86 specific implementation.
 
+* **Added prefetch with intention to write APIs.**
+
+  Added new prefetch function variants e.g. ``rte_prefetch0_write``,
+  which allow the programmer to prefetch a cache line and also indicate
+  the intention to write.
+
 * **Updated CRC modules of the net library.**
 
   * Added runtime selection of the optimal architecture-specific CRC path.
diff --git a/lib/librte_eal/include/generic/rte_prefetch.h b/lib/librte_eal/include/generic/rte_prefetch.h
index 6e47bdfbad..53d68c40f1 100644
--- a/lib/librte_eal/include/generic/rte_prefetch.h
+++ b/lib/librte_eal/include/generic/rte_prefetch.h
@@ -5,6 +5,8 @@
 #ifndef _RTE_PREFETCH_H_
 #define _RTE_PREFETCH_H_
 
+#include "rte_compat.h"
+
 /**
  * @file
  *
@@ -51,4 +53,59 @@ static inline void rte_prefetch2(const volatile void *p);
  */
 static inline void rte_prefetch_non_temporal(const volatile void *p);
 
+/**
+ * Prefetch a cache line into all cache levels, with intention to write. This
+ * prefetch variant hints to the CPU that the program is expecting to write to
+ * the cache line being prefetched.
+ *
+ * @param p Address to prefetch
+ */
+__rte_experimental
+static inline void
+rte_prefetch0_write(const void *p)
+{
+	/* 1 indicates intention to write, 3 sets target cache level to L1. See
+	 * GCC docs where these integer constants are described in more detail:
+	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
+	 */
+	__builtin_prefetch(p, 1, 3);
+}
+
+/**
+ * Prefetch a cache line into all cache levels, except the 0th, with intention
+ * to write. This prefetch variant hints to the CPU that the program is
+ * expecting to write to the cache line being prefetched.
+ *
+ * @param p Address to prefetch
+ */
+__rte_experimental
+static inline void
+rte_prefetch1_write(const void *p)
+{
+	/* 1 indicates intention to write, 2 sets target cache level to L2. See
+	 * GCC docs where these integer constants are described in more detail:
+	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
+	 */
+	__builtin_prefetch(p, 1, 2);
+}
+
+/**
+ * Prefetch a cache line into all cache levels, except the 0th and 1st, with
+ * intention to write. This prefetch variant hints to the CPU that the program
+ * is expecting to write to the cache line being prefetched.
+ *
+ * @param p Address to prefetch
+ */
+__rte_experimental
+static inline void
+rte_prefetch2_write(const void *p)
+{
+	/* 1 indicates intention to write, 1 sets target cache level to L3. See
+	 * GCC docs where these integer constants are described in more detail:
+	 *  https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
+	 */
+	__builtin_prefetch(p, 1, 1);
+}
+
+
 #endif /* _RTE_PREFETCH_H_ */
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v3] eal: add new prefetch write variants
  2020-10-15 10:32   ` [dpdk-dev] [PATCH v3] " Harry van Haaren
@ 2020-10-15 20:27     ` David Marchand
  0 siblings, 0 replies; 15+ messages in thread
From: David Marchand @ 2020-10-15 20:27 UTC (permalink / raw)
  To: Harry van Haaren
  Cc: dev, Ruifeng Wang (Arm Technology China),
	Jerin Jacob Kollanukkaran, Pavan Nikhilesh

On Thu, Oct 15, 2020 at 12:31 PM Harry van Haaren
<harry.van.haaren@intel.com> wrote:
>
> This commit adds new rte_prefetchX_write() variants, that suggest to the
> compiler to use a prefetch instruction with intention to write. As a
> compiler builtin, the compiler can choose based on compilation target
> what the best implementation for this instruction is.
>
> Three versions are provided, targeting the different levels of cache.
>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> Reviewed-by: Jerin Jacob <jerinj@marvell.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>

Added experimental banners in doxygen.
Applied, thanks.


-- 
David Marchand


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

end of thread, other threads:[~2020-10-15 20:28 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-11  9:19 [dpdk-dev] [PATCH] eal: add new prefetch0_write variant Harry van Haaren
2020-09-13 20:11 ` Pavan Nikhilesh Bhagavatula
2020-09-14  8:12   ` Van Haaren, Harry
2020-09-14 10:39     ` Pavan Nikhilesh Bhagavatula
2020-09-14 15:10       ` Van Haaren, Harry
2020-09-14 15:10 ` [dpdk-dev] [PATCH v2] eal: add new prefetch write variants Harry van Haaren
2020-10-08  7:42   ` David Marchand
2020-10-08  8:34     ` Van Haaren, Harry
2020-10-08  8:39       ` Van Haaren, Harry
2020-10-08  8:54   ` Jerin Jacob
2020-10-10 10:21   ` Ruifeng Wang
2020-10-15  8:18   ` David Marchand
2020-10-15  8:44     ` Van Haaren, Harry
2020-10-15 10:32   ` [dpdk-dev] [PATCH v3] " Harry van Haaren
2020-10-15 20:27     ` David Marchand

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git