DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
@ 2015-11-26 10:49 Michael Qiu
  2015-11-26 21:28 ` Thomas Monjalon
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Michael Qiu @ 2015-11-26 10:49 UTC (permalink / raw)
  To: dev

gcc 4.3.4 does not include "immintrin.h", and will post below error:
    lib/librte_sched/rte_sched.c:56:23: error:
    immintrin.h: No such file or directory

To avoid this issue, a gcc version check is need and a flag to indicate
vector ablility.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 lib/librte_sched/rte_sched.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index d47cfc2..780b314 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -53,7 +53,21 @@
 #endif
 
 #ifdef RTE_SCHED_VECTOR
+
+#if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
+
+#if defined(__AVX__)
 #include <immintrin.h>
+#define SCHED_VECTOR_ENABLE
+#endif
+
+#else
+
+#include <x86intrin.h>
+#define SCHED_VECTOR_ENABLE
+
+#endif
+
 #endif
 
 #define RTE_SCHED_TB_RATE_CONFIG_ERR          (1e-7)
@@ -1667,7 +1681,7 @@ grinder_schedule(struct rte_sched_port *port, uint32_t pos)
 	return 1;
 }
 
-#ifdef RTE_SCHED_VECTOR
+#ifdef SCHED_VECTOR_ENABLE
 
 static inline int
 grinder_pipe_exists(struct rte_sched_port *port, uint32_t base_pipe)
-- 
1.9.3

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

* Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-11-26 10:49 [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4 Michael Qiu
@ 2015-11-26 21:28 ` Thomas Monjalon
  2015-11-27  2:26   ` Qiu, Michael
  2015-12-02  2:09 ` [dpdk-dev] [PATCH v2] " Michael Qiu
  2015-12-02  2:39 ` [dpdk-dev] [PATCH v3] " Michael Qiu
  2 siblings, 1 reply; 15+ messages in thread
From: Thomas Monjalon @ 2015-11-26 21:28 UTC (permalink / raw)
  To: Michael Qiu; +Cc: dev

2015-11-26 18:49, Michael Qiu:
> gcc 4.3.4 does not include "immintrin.h", and will post below error:
>     lib/librte_sched/rte_sched.c:56:23: error:
>     immintrin.h: No such file or directory
> 
> To avoid this issue, a gcc version check is need and a flag to indicate
> vector ablility.
[...]
> +#if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
> +
> +#if defined(__AVX__)
>  #include <immintrin.h>
> +#define SCHED_VECTOR_ENABLE
> +#endif
> +
> +#else
> +
> +#include <x86intrin.h>
> +#define SCHED_VECTOR_ENABLE
> +
> +#endif

This kind of complication is managed by EAL.
I think we should include rte_vect.h.

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

* Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-11-26 21:28 ` Thomas Monjalon
@ 2015-11-27  2:26   ` Qiu, Michael
  2015-11-27  8:59     ` Thomas Monjalon
  0 siblings, 1 reply; 15+ messages in thread
From: Qiu, Michael @ 2015-11-27  2:26 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On 2015/11/27 5:29, Thomas Monjalon wrote:
> 2015-11-26 18:49, Michael Qiu:
>> gcc 4.3.4 does not include "immintrin.h", and will post below error:
>>     lib/librte_sched/rte_sched.c:56:23: error:
>>     immintrin.h: No such file or directory
>>
>> To avoid this issue, a gcc version check is need and a flag to indicate
>> vector ablility.
> [...]
>> +#if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
>> +
>> +#if defined(__AVX__)
>>  #include <immintrin.h>
>> +#define SCHED_VECTOR_ENABLE
>> +#endif
>> +
>> +#else
>> +
>> +#include <x86intrin.h>
>> +#define SCHED_VECTOR_ENABLE
>> +
>> +#endif
> This kind of complication is managed by EAL.
> I think we should include rte_vect.h.

As I know here it needs a flag to identify whether the platform support
AVX, if not it will not use it, so I don't know if we could only simply
include rte_vect.h?

Thanks,
Michael
>
>


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

* Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-11-27  2:26   ` Qiu, Michael
@ 2015-11-27  8:59     ` Thomas Monjalon
  2015-11-27 11:53       ` Qiu, Michael
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Monjalon @ 2015-11-27  8:59 UTC (permalink / raw)
  To: Qiu, Michael; +Cc: dev

2015-11-27 02:26, Qiu, Michael:
> On 2015/11/27 5:29, Thomas Monjalon wrote:
> > 2015-11-26 18:49, Michael Qiu:
> >> gcc 4.3.4 does not include "immintrin.h", and will post below error:
> >>     lib/librte_sched/rte_sched.c:56:23: error:
> >>     immintrin.h: No such file or directory
> >>
> >> To avoid this issue, a gcc version check is need and a flag to indicate
> >> vector ablility.
> > [...]
> >> +#if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
> >> +
> >> +#if defined(__AVX__)
> >>  #include <immintrin.h>
> >> +#define SCHED_VECTOR_ENABLE
> >> +#endif
> >> +
> >> +#else
> >> +
> >> +#include <x86intrin.h>
> >> +#define SCHED_VECTOR_ENABLE
> >> +
> >> +#endif
> > This kind of complication is managed by EAL.
> > I think we should include rte_vect.h.
> 
> As I know here it needs a flag to identify whether the platform support
> AVX, if not it will not use it, so I don't know if we could only simply
> include rte_vect.h?

It's not exclusive.
You can include rte_vect.h and check AVX to define SCHED_VECTOR_ENABLE.

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

* Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-11-27  8:59     ` Thomas Monjalon
@ 2015-11-27 11:53       ` Qiu, Michael
  2015-11-27 12:34         ` Ananyev, Konstantin
  0 siblings, 1 reply; 15+ messages in thread
From: Qiu, Michael @ 2015-11-27 11:53 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

really?I don't think so.

AVX Marco only exist in the gcc version below 4.4,  I still need to check if below or beyond 4.4 am I right?

Thanks,
Michael


> 在 2015年11月27日,下午5:01,Thomas Monjalon <thomas.monjalon@6wind.com> 写道:
> 
> 2015-11-27 02:26, Qiu, Michael:
>>> On 2015/11/27 5:29, Thomas Monjalon wrote:
>>> 2015-11-26 18:49, Michael Qiu:
>>>> gcc 4.3.4 does not include "immintrin.h", and will post below error:
>>>>    lib/librte_sched/rte_sched.c:56:23: error:
>>>>    immintrin.h: No such file or directory
>>>> 
>>>> To avoid this issue, a gcc version check is need and a flag to indicate
>>>> vector ablility.
>>> [...]
>>>> +#if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
>>>> +
>>>> +#if defined(__AVX__)
>>>> #include <immintrin.h>
>>>> +#define SCHED_VECTOR_ENABLE
>>>> +#endif
>>>> +
>>>> +#else
>>>> +
>>>> +#include <x86intrin.h>
>>>> +#define SCHED_VECTOR_ENABLE
>>>> +
>>>> +#endif
>>> This kind of complication is managed by EAL.
>>> I think we should include rte_vect.h.
>> 
>> As I know here it needs a flag to identify whether the platform support
>> AVX, if not it will not use it, so I don't know if we could only simply
>> include rte_vect.h?
> 
> It's not exclusive.
> You can include rte_vect.h and check AVX to define SCHED_VECTOR_ENABLE.
> 

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

* Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-11-27 11:53       ` Qiu, Michael
@ 2015-11-27 12:34         ` Ananyev, Konstantin
  2015-11-27 14:01           ` Qiu, Michael
  0 siblings, 1 reply; 15+ messages in thread
From: Ananyev, Konstantin @ 2015-11-27 12:34 UTC (permalink / raw)
  To: Qiu, Michael, Thomas Monjalon; +Cc: dev


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiu, Michael
> Sent: Friday, November 27, 2015 11:53 AM
> To: Thomas Monjalon
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
> 
> really?I don't think so.
> 
> AVX Marco only exist in the gcc version below 4.4,  I still need to check if below or beyond 4.4 am I right?
> 
> Thanks,
> Michael


If you look at lib/librte_eal/common/include/arch/x86/rte_vect.h, you'll see the code similar
to one you are trying to put into rte_shed.c:

lib/librte_eal/common/include/arch/x86/rte_vect.h:
...
#if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))

#ifdef __SSE__
#include <xmmintrin.h>
#endif

#ifdef __SSE2__
#include <emmintrin.h>
#endif

#ifdef __SSE3__
#include <tmmintrin.h>
#endif

#if defined(__SSE4_2__) || defined(__SSE4_1__)
#include <smmintrin.h>
#endif

#if defined(__AVX__)
#include <immintrin.h>
#endif

#else

#include <x86intrin.h>

#endif
...

So I think you can do just like that:

#include <rte_vect.h>
#if defined(__AVX__)
#define SCHED_VECTOR_ENABLE
#endif

inside rte_sched.c

Konstantin


> 
> 
> > 在 2015年11月27日,下午5:01,Thomas Monjalon <thomas.monjalon@6wind.com> 写道:
> >
> > 2015-11-27 02:26, Qiu, Michael:
> >>> On 2015/11/27 5:29, Thomas Monjalon wrote:
> >>> 2015-11-26 18:49, Michael Qiu:
> >>>> gcc 4.3.4 does not include "immintrin.h", and will post below error:
> >>>>    lib/librte_sched/rte_sched.c:56:23: error:
> >>>>    immintrin.h: No such file or directory
> >>>>
> >>>> To avoid this issue, a gcc version check is need and a flag to indicate
> >>>> vector ablility.
> >>> [...]
> >>>> +#if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
> >>>> +
> >>>> +#if defined(__AVX__)
> >>>> #include <immintrin.h>
> >>>> +#define SCHED_VECTOR_ENABLE
> >>>> +#endif
> >>>> +
> >>>> +#else
> >>>> +
> >>>> +#include <x86intrin.h>
> >>>> +#define SCHED_VECTOR_ENABLE
> >>>> +
> >>>> +#endif
> >>> This kind of complication is managed by EAL.
> >>> I think we should include rte_vect.h.
> >>
> >> As I know here it needs a flag to identify whether the platform support
> >> AVX, if not it will not use it, so I don't know if we could only simply
> >> include rte_vect.h?
> >
> > It's not exclusive.
> > You can include rte_vect.h and check AVX to define SCHED_VECTOR_ENABLE.
> >

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

* Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-11-27 12:34         ` Ananyev, Konstantin
@ 2015-11-27 14:01           ` Qiu, Michael
  2015-11-27 14:09             ` Ananyev, Konstantin
  0 siblings, 1 reply; 15+ messages in thread
From: Qiu, Michael @ 2015-11-27 14:01 UTC (permalink / raw)
  To: Ananyev, Konstantin; +Cc: dev

I just replied that Marco AVX only exist in gcc version < 4.4 , otherwise it will not exist.

What's your suggest will not work if gcc version greater than 4.3.

So still need to check gcc version. Any other solution?

Thanks,
Michael

> 在 2015年11月27日,下午8:34,Ananyev, Konstantin <konstantin.ananyev@intel.com> 写道:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiu, Michael
>> Sent: Friday, November 27, 2015 11:53 AM
>> To: Thomas Monjalon
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
>> 
>> really?I don't think so.
>> 
>> AVX Marco only exist in the gcc version below 4.4,  I still need to check if below or beyond 4.4 am I right?
>> 
>> Thanks,
>> Michael
> 
> 
> If you look at lib/librte_eal/common/include/arch/x86/rte_vect.h, you'll see the code similar
> to one you are trying to put into rte_shed.c:
> 
> lib/librte_eal/common/include/arch/x86/rte_vect.h:
> ...
> #if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
> 
> #ifdef __SSE__
> #include <xmmintrin.h>
> #endif
> 
> #ifdef __SSE2__
> #include <emmintrin.h>
> #endif
> 
> #ifdef __SSE3__
> #include <tmmintrin.h>
> #endif
> 
> #if defined(__SSE4_2__) || defined(__SSE4_1__)
> #include <smmintrin.h>
> #endif
> 
> #if defined(__AVX__)
> #include <immintrin.h>
> #endif
> 
> #else
> 
> #include <x86intrin.h>
> 
> #endif
> ...
> 
> So I think you can do just like that:
> 
> #include <rte_vect.h>
> #if defined(__AVX__)
> #define SCHED_VECTOR_ENABLE
> #endif
> 
> inside rte_sched.c
> 
> Konstantin
> 
> 
>> 
>> 
>>> 在 2015年11月27日,下午5:01,Thomas Monjalon <thomas.monjalon@6wind.com> 写道:
>>> 
>>> 2015-11-27 02:26, Qiu, Michael:
>>>>>> On 2015/11/27 5:29, Thomas Monjalon wrote:
>>>>>> 2015-11-26 18:49, Michael Qiu:
>>>>>> gcc 4.3.4 does not include "immintrin.h", and will post below error:
>>>>>>   lib/librte_sched/rte_sched.c:56:23: error:
>>>>>>   immintrin.h: No such file or directory
>>>>>> 
>>>>>> To avoid this issue, a gcc version check is need and a flag to indicate
>>>>>> vector ablility.
>>>>> [...]
>>>>>> +#if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
>>>>>> +
>>>>>> +#if defined(__AVX__)
>>>>>> #include <immintrin.h>
>>>>>> +#define SCHED_VECTOR_ENABLE
>>>>>> +#endif
>>>>>> +
>>>>>> +#else
>>>>>> +
>>>>>> +#include <x86intrin.h>
>>>>>> +#define SCHED_VECTOR_ENABLE
>>>>>> +
>>>>>> +#endif
>>>>> This kind of complication is managed by EAL.
>>>>> I think we should include rte_vect.h.
>>>> 
>>>> As I know here it needs a flag to identify whether the platform support
>>>> AVX, if not it will not use it, so I don't know if we could only simply
>>>> include rte_vect.h?
>>> 
>>> It's not exclusive.
>>> You can include rte_vect.h and check AVX to define SCHED_VECTOR_ENABLE.
>>> 

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

* Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-11-27 14:01           ` Qiu, Michael
@ 2015-11-27 14:09             ` Ananyev, Konstantin
  2015-11-27 15:22               ` Qiu, Michael
  0 siblings, 1 reply; 15+ messages in thread
From: Ananyev, Konstantin @ 2015-11-27 14:09 UTC (permalink / raw)
  To: Qiu, Michael; +Cc: dev



> -----Original Message-----
> From: Qiu, Michael
> Sent: Friday, November 27, 2015 2:02 PM
> To: Ananyev, Konstantin
> Cc: Thomas Monjalon; dev@dpdk.org
> Subject: Re: [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
> 
> I just replied that Marco AVX only exist in gcc version < 4.4 , otherwise it will not exist.

If macro __AVX__ not defined, then 
#if defined(__AVX__)
would always be false and SCHED_VECTOR_ENABLE also wouldn't be defined.
So still don't understand why that is a problem
Konstantin

> 
> What's your suggest will not work if gcc version greater than 4.3.
> 
> So still need to check gcc version. Any other solution?
> 
> Thanks,
> Michael
> 
> > 在 2015年11月27日,下午8:34,Ananyev, Konstantin <konstantin.ananyev@intel.com> 写道:
> >
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiu, Michael
> >> Sent: Friday, November 27, 2015 11:53 AM
> >> To: Thomas Monjalon
> >> Cc: dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
> >>
> >> really?I don't think so.
> >>
> >> AVX Marco only exist in the gcc version below 4.4,  I still need to check if below or beyond 4.4 am I right?
> >>
> >> Thanks,
> >> Michael
> >
> >
> > If you look at lib/librte_eal/common/include/arch/x86/rte_vect.h, you'll see the code similar
> > to one you are trying to put into rte_shed.c:
> >
> > lib/librte_eal/common/include/arch/x86/rte_vect.h:
> > ...
> > #if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
> >
> > #ifdef __SSE__
> > #include <xmmintrin.h>
> > #endif
> >
> > #ifdef __SSE2__
> > #include <emmintrin.h>
> > #endif
> >
> > #ifdef __SSE3__
> > #include <tmmintrin.h>
> > #endif
> >
> > #if defined(__SSE4_2__) || defined(__SSE4_1__)
> > #include <smmintrin.h>
> > #endif
> >
> > #if defined(__AVX__)
> > #include <immintrin.h>
> > #endif
> >
> > #else
> >
> > #include <x86intrin.h>
> >
> > #endif
> > ...
> >
> > So I think you can do just like that:
> >
> > #include <rte_vect.h>
> > #if defined(__AVX__)
> > #define SCHED_VECTOR_ENABLE
> > #endif
> >
> > inside rte_sched.c
> >
> > Konstantin
> >
> >
> >>
> >>
> >>> 在 2015年11月27日,下午5:01,Thomas Monjalon <thomas.monjalon@6wind.com> 写道:
> >>>
> >>> 2015-11-27 02:26, Qiu, Michael:
> >>>>>> On 2015/11/27 5:29, Thomas Monjalon wrote:
> >>>>>> 2015-11-26 18:49, Michael Qiu:
> >>>>>> gcc 4.3.4 does not include "immintrin.h", and will post below error:
> >>>>>>   lib/librte_sched/rte_sched.c:56:23: error:
> >>>>>>   immintrin.h: No such file or directory
> >>>>>>
> >>>>>> To avoid this issue, a gcc version check is need and a flag to indicate
> >>>>>> vector ablility.
> >>>>> [...]
> >>>>>> +#if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
> >>>>>> +
> >>>>>> +#if defined(__AVX__)
> >>>>>> #include <immintrin.h>
> >>>>>> +#define SCHED_VECTOR_ENABLE
> >>>>>> +#endif
> >>>>>> +
> >>>>>> +#else
> >>>>>> +
> >>>>>> +#include <x86intrin.h>
> >>>>>> +#define SCHED_VECTOR_ENABLE
> >>>>>> +
> >>>>>> +#endif
> >>>>> This kind of complication is managed by EAL.
> >>>>> I think we should include rte_vect.h.
> >>>>
> >>>> As I know here it needs a flag to identify whether the platform support
> >>>> AVX, if not it will not use it, so I don't know if we could only simply
> >>>> include rte_vect.h?
> >>>
> >>> It's not exclusive.
> >>> You can include rte_vect.h and check AVX to define SCHED_VECTOR_ENABLE.
> >>>

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

* Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-11-27 14:09             ` Ananyev, Konstantin
@ 2015-11-27 15:22               ` Qiu, Michael
  2015-11-27 16:21                 ` Ananyev, Konstantin
  0 siblings, 1 reply; 15+ messages in thread
From: Qiu, Michael @ 2015-11-27 15:22 UTC (permalink / raw)
  To: Ananyev, Konstantin; +Cc: dev

Sorry for not explaining clearly. 

For gcc version start from version 4.4, x86intrin.h will be include, and inside x86intrin.h, immintrin.h will be directly include without check AVX (as I know, AVX is not exist when gcc >= 4.4),so no AVX macro does not mean vector disable.

Only gcc < 4.4 and no macro AVX will disable vector.

This is my understanding, may be wrong :)

Thanks,
Michael

> 在 2015年11月27日,下午10:09,Ananyev, Konstantin <konstantin.ananyev@intel.com> 写道:
> 
> 
> 
>> -----Original Message-----
>> From: Qiu, Michael
>> Sent: Friday, November 27, 2015 2:02 PM
>> To: Ananyev, Konstantin
>> Cc: Thomas Monjalon; dev@dpdk.org
>> Subject: Re: [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
>> 
>> I just replied that Marco AVX only exist in gcc version < 4.4 , otherwise it will not exist.
> 
> If macro __AVX__ not defined, then 
> #if defined(__AVX__)
> would always be false and SCHED_VECTOR_ENABLE also wouldn't be defined.
> So still don't understand why that is a problem
> Konstantin
> 
>> 
>> What's your suggest will not work if gcc version greater than 4.3.
>> 
>> So still need to check gcc version. Any other solution?
>> 
>> Thanks,
>> Michael
>> 
>>> 在 2015年11月27日,下午8:34,Ananyev, Konstantin <konstantin.ananyev@intel.com> 写道:
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiu, Michael
>>>> Sent: Friday, November 27, 2015 11:53 AM
>>>> To: Thomas Monjalon
>>>> Cc: dev@dpdk.org
>>>> Subject: Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
>>>> 
>>>> really?I don't think so.
>>>> 
>>>> AVX Marco only exist in the gcc version below 4.4,  I still need to check if below or beyond 4.4 am I right?
>>>> 
>>>> Thanks,
>>>> Michael
>>> 
>>> 
>>> If you look at lib/librte_eal/common/include/arch/x86/rte_vect.h, you'll see the code similar
>>> to one you are trying to put into rte_shed.c:
>>> 
>>> lib/librte_eal/common/include/arch/x86/rte_vect.h:
>>> ...
>>> #if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
>>> 
>>> #ifdef __SSE__
>>> #include <xmmintrin.h>
>>> #endif
>>> 
>>> #ifdef __SSE2__
>>> #include <emmintrin.h>
>>> #endif
>>> 
>>> #ifdef __SSE3__
>>> #include <tmmintrin.h>
>>> #endif
>>> 
>>> #if defined(__SSE4_2__) || defined(__SSE4_1__)
>>> #include <smmintrin.h>
>>> #endif
>>> 
>>> #if defined(__AVX__)
>>> #include <immintrin.h>
>>> #endif
>>> 
>>> #else
>>> 
>>> #include <x86intrin.h>
>>> 
>>> #endif
>>> ...
>>> 
>>> So I think you can do just like that:
>>> 
>>> #include <rte_vect.h>
>>> #if defined(__AVX__)
>>> #define SCHED_VECTOR_ENABLE
>>> #endif
>>> 
>>> inside rte_sched.c
>>> 
>>> Konstantin
>>> 
>>> 
>>>> 
>>>> 
>>>>> 在 2015年11月27日,下午5:01,Thomas Monjalon <thomas.monjalon@6wind.com> 写道:
>>>>> 
>>>>> 2015-11-27 02:26, Qiu, Michael:
>>>>>>>> On 2015/11/27 5:29, Thomas Monjalon wrote:
>>>>>>>> 2015-11-26 18:49, Michael Qiu:
>>>>>>>> gcc 4.3.4 does not include "immintrin.h", and will post below error:
>>>>>>>>  lib/librte_sched/rte_sched.c:56:23: error:
>>>>>>>>  immintrin.h: No such file or directory
>>>>>>>> 
>>>>>>>> To avoid this issue, a gcc version check is need and a flag to indicate
>>>>>>>> vector ablility.
>>>>>>> [...]
>>>>>>>> +#if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
>>>>>>>> +
>>>>>>>> +#if defined(__AVX__)
>>>>>>>> #include <immintrin.h>
>>>>>>>> +#define SCHED_VECTOR_ENABLE
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>> +#else
>>>>>>>> +
>>>>>>>> +#include <x86intrin.h>
>>>>>>>> +#define SCHED_VECTOR_ENABLE
>>>>>>>> +
>>>>>>>> +#endif
>>>>>>> This kind of complication is managed by EAL.
>>>>>>> I think we should include rte_vect.h.
>>>>>> 
>>>>>> As I know here it needs a flag to identify whether the platform support
>>>>>> AVX, if not it will not use it, so I don't know if we could only simply
>>>>>> include rte_vect.h?
>>>>> 
>>>>> It's not exclusive.
>>>>> You can include rte_vect.h and check AVX to define SCHED_VECTOR_ENABLE.
>>>>> 

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

* Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-11-27 15:22               ` Qiu, Michael
@ 2015-11-27 16:21                 ` Ananyev, Konstantin
  0 siblings, 0 replies; 15+ messages in thread
From: Ananyev, Konstantin @ 2015-11-27 16:21 UTC (permalink / raw)
  To: Qiu, Michael; +Cc: dev



> -----Original Message-----
> From: Qiu, Michael
> Sent: Friday, November 27, 2015 3:22 PM
> To: Ananyev, Konstantin
> Cc: Thomas Monjalon; dev@dpdk.org
> Subject: Re: [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
> 
> Sorry for not explaining clearly.
> 
> For gcc version start from version 4.4, x86intrin.h will be include, and inside x86intrin.h, immintrin.h will be directly include without
> check AVX

Yes, but inside immintrin.h there is a check.
At least that what I am seeing for gcc4.4.3:

#ifdef __AVX__
#include <avxintrin.h>
#endif

Isn't it the same for your case? 
Konstantin

> (as I know, AVX is not exist when gcc >= 4.4),so no AVX macro does not mean vector disable.
> 
> Only gcc < 4.4 and no macro AVX will disable vector.
> 
> This is my understanding, may be wrong :)
> 
> Thanks,
> Michael
> 
> > 在 2015年11月27日,下午10:09,Ananyev, Konstantin <konstantin.ananyev@intel.com> 写道:
> >
> >
> >
> >> -----Original Message-----
> >> From: Qiu, Michael
> >> Sent: Friday, November 27, 2015 2:02 PM
> >> To: Ananyev, Konstantin
> >> Cc: Thomas Monjalon; dev@dpdk.org
> >> Subject: Re: [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
> >>
> >> I just replied that Marco AVX only exist in gcc version < 4.4 , otherwise it will not exist.
> >
> > If macro __AVX__ not defined, then
> > #if defined(__AVX__)
> > would always be false and SCHED_VECTOR_ENABLE also wouldn't be defined.
> > So still don't understand why that is a problem
> > Konstantin
> >
> >>
> >> What's your suggest will not work if gcc version greater than 4.3.
> >>
> >> So still need to check gcc version. Any other solution?
> >>
> >> Thanks,
> >> Michael
> >>
> >>> 在 2015年11月27日,下午8:34,Ananyev, Konstantin <konstantin.ananyev@intel.com> 写道:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiu, Michael
> >>>> Sent: Friday, November 27, 2015 11:53 AM
> >>>> To: Thomas Monjalon
> >>>> Cc: dev@dpdk.org
> >>>> Subject: Re: [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4
> >>>>
> >>>> really?I don't think so.
> >>>>
> >>>> AVX Marco only exist in the gcc version below 4.4,  I still need to check if below or beyond 4.4 am I right?
> >>>>
> >>>> Thanks,
> >>>> Michael
> >>>
> >>>
> >>> If you look at lib/librte_eal/common/include/arch/x86/rte_vect.h, you'll see the code similar
> >>> to one you are trying to put into rte_shed.c:
> >>>
> >>> lib/librte_eal/common/include/arch/x86/rte_vect.h:
> >>> ...
> >>> #if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
> >>>
> >>> #ifdef __SSE__
> >>> #include <xmmintrin.h>
> >>> #endif
> >>>
> >>> #ifdef __SSE2__
> >>> #include <emmintrin.h>
> >>> #endif
> >>>
> >>> #ifdef __SSE3__
> >>> #include <tmmintrin.h>
> >>> #endif
> >>>
> >>> #if defined(__SSE4_2__) || defined(__SSE4_1__)
> >>> #include <smmintrin.h>
> >>> #endif
> >>>
> >>> #if defined(__AVX__)
> >>> #include <immintrin.h>
> >>> #endif
> >>>
> >>> #else
> >>>
> >>> #include <x86intrin.h>
> >>>
> >>> #endif
> >>> ...
> >>>
> >>> So I think you can do just like that:
> >>>
> >>> #include <rte_vect.h>
> >>> #if defined(__AVX__)
> >>> #define SCHED_VECTOR_ENABLE
> >>> #endif
> >>>
> >>> inside rte_sched.c
> >>>
> >>> Konstantin
> >>>
> >>>
> >>>>
> >>>>
> >>>>> 在 2015年11月27日,下午5:01,Thomas Monjalon <thomas.monjalon@6wind.com> 写道:
> >>>>>
> >>>>> 2015-11-27 02:26, Qiu, Michael:
> >>>>>>>> On 2015/11/27 5:29, Thomas Monjalon wrote:
> >>>>>>>> 2015-11-26 18:49, Michael Qiu:
> >>>>>>>> gcc 4.3.4 does not include "immintrin.h", and will post below error:
> >>>>>>>>  lib/librte_sched/rte_sched.c:56:23: error:
> >>>>>>>>  immintrin.h: No such file or directory
> >>>>>>>>
> >>>>>>>> To avoid this issue, a gcc version check is need and a flag to indicate
> >>>>>>>> vector ablility.
> >>>>>>> [...]
> >>>>>>>> +#if (defined(__ICC) || (__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
> >>>>>>>> +
> >>>>>>>> +#if defined(__AVX__)
> >>>>>>>> #include <immintrin.h>
> >>>>>>>> +#define SCHED_VECTOR_ENABLE
> >>>>>>>> +#endif
> >>>>>>>> +
> >>>>>>>> +#else
> >>>>>>>> +
> >>>>>>>> +#include <x86intrin.h>
> >>>>>>>> +#define SCHED_VECTOR_ENABLE
> >>>>>>>> +
> >>>>>>>> +#endif
> >>>>>>> This kind of complication is managed by EAL.
> >>>>>>> I think we should include rte_vect.h.
> >>>>>>
> >>>>>> As I know here it needs a flag to identify whether the platform support
> >>>>>> AVX, if not it will not use it, so I don't know if we could only simply
> >>>>>> include rte_vect.h?
> >>>>>
> >>>>> It's not exclusive.
> >>>>> You can include rte_vect.h and check AVX to define SCHED_VECTOR_ENABLE.
> >>>>>

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

* [dpdk-dev] [PATCH v2] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-11-26 10:49 [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4 Michael Qiu
  2015-11-26 21:28 ` Thomas Monjalon
@ 2015-12-02  2:09 ` Michael Qiu
  2015-12-02  2:18   ` Thomas Monjalon
  2015-12-02  2:39 ` [dpdk-dev] [PATCH v3] " Michael Qiu
  2 siblings, 1 reply; 15+ messages in thread
From: Michael Qiu @ 2015-12-02  2:09 UTC (permalink / raw)
  To: dev

gcc 4.3.4 does not include "immintrin.h", and will post below error:
    lib/librte_sched/rte_sched.c:56:23: error:
    immintrin.h: No such file or directory

To avoid this issue, a gcc version check is need and a flag to indicate
vector ablility.

Fixes: 42ec27a0178a ("sched: enable SSE optimizations in config")

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
v2 --> v1:
	include header file rte_vect.h instead of gcc version check
	change __AVX__ to __SSE2__ since all vector function are SSE2 related.

 lib/librte_sched/rte_sched.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index d47cfc2..0d46618 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -42,6 +42,7 @@
 #include <rte_prefetch.h>
 #include <rte_branch_prediction.h>
 #include <rte_mbuf.h>
+#include <rte_vect.h>
 
 #include "rte_sched.h"
 #include "rte_bitmap.h"
@@ -53,7 +54,11 @@
 #endif
 
 #ifdef RTE_SCHED_VECTOR
-#include <immintrin.h>
+
+#if defined(__SSE2__)
+#define SCHED_VECTOR_ENABLE
+#endif
+
 #endif
 
 #define RTE_SCHED_TB_RATE_CONFIG_ERR          (1e-7)
@@ -1667,7 +1672,7 @@ grinder_schedule(struct rte_sched_port *port, uint32_t pos)
 	return 1;
 }
 
-#ifdef RTE_SCHED_VECTOR
+#ifdef SCHED_VECTOR_ENABLE
 
 static inline int
 grinder_pipe_exists(struct rte_sched_port *port, uint32_t base_pipe)
-- 
1.9.3

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

* Re: [dpdk-dev] [PATCH v2] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-12-02  2:09 ` [dpdk-dev] [PATCH v2] " Michael Qiu
@ 2015-12-02  2:18   ` Thomas Monjalon
  2015-12-02  2:29     ` Qiu, Michael
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Monjalon @ 2015-12-02  2:18 UTC (permalink / raw)
  To: Michael Qiu; +Cc: dev

2015-12-02 10:09, Michael Qiu:
> gcc 4.3.4 does not include "immintrin.h", and will post below error:
>     lib/librte_sched/rte_sched.c:56:23: error:
>     immintrin.h: No such file or directory

This compiler issue is fixed with rte_vect.h.

> To avoid this issue, a gcc version check is need and a flag to indicate
> vector ablility.

It is another issue: we need SSE2 support.

> --- a/lib/librte_sched/rte_sched.c
> +++ b/lib/librte_sched/rte_sched.c
> @@ -42,6 +42,7 @@
>  #include <rte_prefetch.h>
>  #include <rte_branch_prediction.h>
>  #include <rte_mbuf.h>
> +#include <rte_vect.h>

Shouldn't be in #ifdef RTE_SCHED_VECTOR ?

>  #include "rte_sched.h"
>  #include "rte_bitmap.h"
> @@ -53,7 +54,11 @@
>  #endif
>  
>  #ifdef RTE_SCHED_VECTOR
> -#include <immintrin.h>
> +
> +#if defined(__SSE2__)
> +#define SCHED_VECTOR_ENABLE
> +#endif

I think the flag should SCHED_VECTOR_SSE2

With this fix, the need for disabling SCHED_VECTOR for non-x86 platforms
should disappear.
But it may be safe to disable it (another patch).
Thanks

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

* Re: [dpdk-dev] [PATCH v2] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-12-02  2:18   ` Thomas Monjalon
@ 2015-12-02  2:29     ` Qiu, Michael
  0 siblings, 0 replies; 15+ messages in thread
From: Qiu, Michael @ 2015-12-02  2:29 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

I will make v3 patch to fix the issue.

Thanks,
Michael

On 2015/12/2 10:19, Thomas Monjalon wrote:
> 2015-12-02 10:09, Michael Qiu:
>> gcc 4.3.4 does not include "immintrin.h", and will post below error:
>>     lib/librte_sched/rte_sched.c:56:23: error:
>>     immintrin.h: No such file or directory
> This compiler issue is fixed with rte_vect.h.
>
>> To avoid this issue, a gcc version check is need and a flag to indicate
>> vector ablility.
> It is another issue: we need SSE2 support.
>
>> --- a/lib/librte_sched/rte_sched.c
>> +++ b/lib/librte_sched/rte_sched.c
>> @@ -42,6 +42,7 @@
>>  #include <rte_prefetch.h>
>>  #include <rte_branch_prediction.h>
>>  #include <rte_mbuf.h>
>> +#include <rte_vect.h>
> Shouldn't be in #ifdef RTE_SCHED_VECTOR ?
>
>>  #include "rte_sched.h"
>>  #include "rte_bitmap.h"
>> @@ -53,7 +54,11 @@
>>  #endif
>>  
>>  #ifdef RTE_SCHED_VECTOR
>> -#include <immintrin.h>
>> +
>> +#if defined(__SSE2__)
>> +#define SCHED_VECTOR_ENABLE
>> +#endif
> I think the flag should SCHED_VECTOR_SSE2
>
> With this fix, the need for disabling SCHED_VECTOR for non-x86 platforms
> should disappear.
> But it may be safe to disable it (another patch).
> Thanks
>


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

* [dpdk-dev] [PATCH v3] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-11-26 10:49 [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4 Michael Qiu
  2015-11-26 21:28 ` Thomas Monjalon
  2015-12-02  2:09 ` [dpdk-dev] [PATCH v2] " Michael Qiu
@ 2015-12-02  2:39 ` Michael Qiu
  2015-12-02 22:10   ` Thomas Monjalon
  2 siblings, 1 reply; 15+ messages in thread
From: Michael Qiu @ 2015-12-02  2:39 UTC (permalink / raw)
  To: dev

gcc 4.3.4 does not include "immintrin.h", and will post below error:
    lib/librte_sched/rte_sched.c:56:23: error:
    immintrin.h: No such file or directory

This compiler issue is fixed with rte_vect.h

There is another issue, need SSE2 support

Fixes: 42ec27a0178a ("sched: enable SSE optimizations in config")

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
v3 --> v2:
	reformat commit log
	move rte_vect.h inside RTE_SCHED_VECTOR

v2 --> v1:
	include header file rte_vect.h instead of gcc version check
	change __AVX__ to __SSE2__ since all vector function are SSE2 related

 lib/librte_sched/rte_sched.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index d47cfc2..21ebf25 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -53,7 +53,12 @@
 #endif
 
 #ifdef RTE_SCHED_VECTOR
-#include <immintrin.h>
+#include <rte_vect.h>
+
+#if defined(__SSE2__)
+#define SCHED_VECTOR_SSE2
+#endif
+
 #endif
 
 #define RTE_SCHED_TB_RATE_CONFIG_ERR          (1e-7)
@@ -1667,7 +1672,7 @@ grinder_schedule(struct rte_sched_port *port, uint32_t pos)
 	return 1;
 }
 
-#ifdef RTE_SCHED_VECTOR
+#ifdef SCHED_VECTOR_SSE2
 
 static inline int
 grinder_pipe_exists(struct rte_sched_port *port, uint32_t base_pipe)
-- 
1.9.3

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

* Re: [dpdk-dev] [PATCH v3] lib/librte_sched: Fix compile with gcc 4.3.4
  2015-12-02  2:39 ` [dpdk-dev] [PATCH v3] " Michael Qiu
@ 2015-12-02 22:10   ` Thomas Monjalon
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Monjalon @ 2015-12-02 22:10 UTC (permalink / raw)
  To: Michael Qiu; +Cc: dev

2015-12-02 10:39, Michael Qiu:
> gcc 4.3.4 does not include "immintrin.h", and will post below error:
>     lib/librte_sched/rte_sched.c:56:23: error:
>     immintrin.h: No such file or directory
> 
> This compiler issue is fixed with rte_vect.h
> 
> There is another issue, need SSE2 support
> 
> Fixes: 42ec27a0178a ("sched: enable SSE optimizations in config")
> 
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>

Applied, thanks

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

end of thread, other threads:[~2015-12-02 22:10 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-26 10:49 [dpdk-dev] [PATCH] lib/librte_sched: Fix compile with gcc 4.3.4 Michael Qiu
2015-11-26 21:28 ` Thomas Monjalon
2015-11-27  2:26   ` Qiu, Michael
2015-11-27  8:59     ` Thomas Monjalon
2015-11-27 11:53       ` Qiu, Michael
2015-11-27 12:34         ` Ananyev, Konstantin
2015-11-27 14:01           ` Qiu, Michael
2015-11-27 14:09             ` Ananyev, Konstantin
2015-11-27 15:22               ` Qiu, Michael
2015-11-27 16:21                 ` Ananyev, Konstantin
2015-12-02  2:09 ` [dpdk-dev] [PATCH v2] " Michael Qiu
2015-12-02  2:18   ` Thomas Monjalon
2015-12-02  2:29     ` Qiu, Michael
2015-12-02  2:39 ` [dpdk-dev] [PATCH v3] " Michael Qiu
2015-12-02 22:10   ` Thomas Monjalon

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