[dpdk-dev] [PATCH] fm10k: fix the compilation on big endian platforms

[dpdk-dev] [PATCH] fm10k: fix the compilation on big endian platforms

[Chao Zhu]

The using of rte_cpu_to_le_32() in pre-compile macros will cause error
'initializer element is not constant' on big endian platforms. This patch
fixes the compilation error of fm10k driver.


Chao Zhu (1):
  fm10k: fix the compilation on big endian platforms

 drivers/net/fm10k/base/fm10k_tlv.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

[dpdk-dev] [PATCH] fm10k: fix the compilation on big endian platforms

[Chao Zhu]

The rte_cpu_to_le_32 function can't be used to define const variables
because it has different implementation on big endian platforms. If
doing so, it will cause 'initializer element is not constant' compiling
error. This patch fixes this problem.

Signed-off-by: Chao Zhu <chaozhu@linux.vnet.ibm.com>
---
 drivers/net/fm10k/base/fm10k_tlv.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/fm10k/base/fm10k_tlv.c b/drivers/net/fm10k/base/fm10k_tlv.c
index 1d9d7d8..5b2937d 100644
--- a/drivers/net/fm10k/base/fm10k_tlv.c
+++ b/drivers/net/fm10k/base/fm10k_tlv.c
@@ -664,8 +664,11 @@ STATIC const s64 test_s64 = -0x123456789abcdef0ll;
 STATIC const s32 test_s32 = -0x1235678;
 STATIC const s16 test_s16 = -0x1234;
 STATIC const s8  test_s8  = -0x12;
-STATIC const __le32 test_le[2] = { FM10K_CPU_TO_LE32(0x12345678),
-				   FM10K_CPU_TO_LE32(0x9abcdef0)};
+#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+STATIC const __le32 test_le[2] = {0x78563412,0xf0debc9a};
+#else
+STATIC const __le32 test_le[2] = {0x12345678,0x9abcdef0};
+#endif
 
 /* The message below is meant to be used as a test message to demonstrate
  * how to use the TLV interface and to test the types.  Normally this code
-- 
1.7.1

Re: [dpdk-dev] [PATCH] fm10k: fix the compilation on big endian platforms

[Thomas Monjalon]

2015-08-03 14:31, Chao Zhu:
> The using of rte_cpu_to_le_32() in pre-compile macros will cause error
> 'initializer element is not constant' on big endian platforms. This patch
> fixes the compilation error of fm10k driver.
> 
> 
> Chao Zhu (1):
>   fm10k: fix the compilation on big endian platforms

Chao, no need of cover-letter for only 1 patch.

Re: [dpdk-dev] [PATCH] fm10k: fix the compilation on big endian platforms

[Chao Zhu]

Got it. Thanks!

On 2015/8/3 16:53, Thomas Monjalon wrote:
> 2015-08-03 14:31, Chao Zhu:
>> The using of rte_cpu_to_le_32() in pre-compile macros will cause error
>> 'initializer element is not constant' on big endian platforms. This patch
>> fixes the compilation error of fm10k driver.
>>
>>
>> Chao Zhu (1):
>>    fm10k: fix the compilation on big endian platforms
> Chao, no need of cover-letter for only 1 patch.
>

Re: [dpdk-dev] [PATCH] fm10k: fix the compilation on big endian platforms

[Thomas Monjalon]

2015-08-03 14:31, Chao Zhu:
> The rte_cpu_to_le_32 function can't be used to define const variables
> because it has different implementation on big endian platforms. If
> doing so, it will cause 'initializer element is not constant' compiling
> error. This patch fixes this problem.

> --- a/drivers/net/fm10k/base/fm10k_tlv.c
> +++ b/drivers/net/fm10k/base/fm10k_tlv.c

You cannot change a base driver file except the osdep header where
FM10K_CPU_TO_LE32 is defined.

I don't understand why it doesn't give you a constant, given this definition:

#define rte_bswap32(x) ((uint32_t)(__builtin_constant_p(x) ?        \                                            
                   rte_constant_bswap32(x) :        \
                   rte_arch_bswap32(x)))

Have you tried CONFIG_RTE_FORCE_INTRINSICS=y ?
It should trigger this definition:

#define rte_bswap32(x) __builtin_bswap32(x)

> -STATIC const __le32 test_le[2] = { FM10K_CPU_TO_LE32(0x12345678),
> -				   FM10K_CPU_TO_LE32(0x9abcdef0)};
> +#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +STATIC const __le32 test_le[2] = {0x78563412,0xf0debc9a};
> +#else
> +STATIC const __le32 test_le[2] = {0x12345678,0x9abcdef0};
> +#endif

Re: [dpdk-dev] [PATCH] fm10k: fix the compilation on big endian platforms

[Chao Zhu]

On 2015/8/3 17:06, Thomas Monjalon wrote:
> 2015-08-03 14:31, Chao Zhu:
>> The rte_cpu_to_le_32 function can't be used to define const variables
>> because it has different implementation on big endian platforms. If
>> doing so, it will cause 'initializer element is not constant' compiling
>> error. This patch fixes this problem.
>> --- a/drivers/net/fm10k/base/fm10k_tlv.c
>> +++ b/drivers/net/fm10k/base/fm10k_tlv.c
> You cannot change a base driver file except the osdep header where
> FM10K_CPU_TO_LE32 is defined.
>
> I don't understand why it doesn't give you a constant, given this definition:
>
> #define rte_bswap32(x) ((uint32_t)(__builtin_constant_p(x) ?        \
>                     rte_constant_bswap32(x) :        \
>                     rte_arch_bswap32(x)))
>
> Have you tried CONFIG_RTE_FORCE_INTRINSICS=y ?
> It should trigger this definition:
>
> #define rte_bswap32(x) __builtin_bswap32(x)
Yes.  CONFIG_RTE_FORCE_INTRINSICS=y works on Power Big endian.  But if I 
turn off this, the error happens.  Seems rte_constant_bswap32 doesn't 
work on Power8? I'll try to check it.
>
>> -STATIC const __le32 test_le[2] = { FM10K_CPU_TO_LE32(0x12345678),
>> -				   FM10K_CPU_TO_LE32(0x9abcdef0)};
>> +#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
>> +STATIC const __le32 test_le[2] = {0x78563412,0xf0debc9a};
>> +#else
>> +STATIC const __le32 test_le[2] = {0x12345678,0x9abcdef0};
>> +#endif

Re: [dpdk-dev] [PATCH] fm10k: fix the compilation on big endian platforms

[Thomas Monjalon]

2015-08-03 19:32, Chao Zhu:
> On 2015/8/3 17:06, Thomas Monjalon wrote:
> > 2015-08-03 14:31, Chao Zhu:
> >> The rte_cpu_to_le_32 function can't be used to define const variables
> >> because it has different implementation on big endian platforms. If
> >> doing so, it will cause 'initializer element is not constant' compiling
> >> error. This patch fixes this problem.
> >> --- a/drivers/net/fm10k/base/fm10k_tlv.c
> >> +++ b/drivers/net/fm10k/base/fm10k_tlv.c
> > 
> > You cannot change a base driver file except the osdep header where
> > FM10K_CPU_TO_LE32 is defined.
> >
> > I don't understand why it doesn't give you a constant, given this definition:
> >
> > #define rte_bswap32(x) ((uint32_t)(__builtin_constant_p(x) ?        \
> >                     rte_constant_bswap32(x) :        \
> >                     rte_arch_bswap32(x)))
> >
> > Have you tried CONFIG_RTE_FORCE_INTRINSICS=y ?
> > It should trigger this definition:
> >
> > #define rte_bswap32(x) __builtin_bswap32(x)
> 
> Yes.  CONFIG_RTE_FORCE_INTRINSICS=y works on Power Big endian.  But if I 
> turn off this, the error happens.  Seems rte_constant_bswap32 doesn't 
> work on Power8? I'll try to check it.

The tile arch force intrinsics:
	http://dpdk.org/browse/dpdk/tree/config/defconfig_tile-tilegx-linuxapp-gcc#n39
	http://dpdk.org/browse/dpdk/tree/lib/librte_eal/common/include/arch/tile/rte_byteorder.h#n36
I don't know wether it was for the same reason.

Re: [dpdk-dev] [PATCH] fm10k: fix the compilation on big endian platforms

[Tony Lu]

>-----Original Message-----
>From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
>Sent: Monday, August 03, 2015 7:39 PM
>To: Chao Zhu
>Cc: dev@dpdk.org; Zhigang Lu
>Subject: Re: [dpdk-dev] [PATCH] fm10k: fix the compilation on big endian
>platforms
>
>2015-08-03 19:32, Chao Zhu:
>> On 2015/8/3 17:06, Thomas Monjalon wrote:
>> > 2015-08-03 14:31, Chao Zhu:
>> >> The rte_cpu_to_le_32 function can't be used to define const
>> >> variables because it has different implementation on big endian
>> >> platforms. If doing so, it will cause 'initializer element is not
>> >> constant' compiling error. This patch fixes this problem.
>> >> --- a/drivers/net/fm10k/base/fm10k_tlv.c
>> >> +++ b/drivers/net/fm10k/base/fm10k_tlv.c
>> >
>> > You cannot change a base driver file except the osdep header where
>> > FM10K_CPU_TO_LE32 is defined.
>> >
>> > I don't understand why it doesn't give you a constant, given this
definition:
>> >
>> > #define rte_bswap32(x) ((uint32_t)(__builtin_constant_p(x) ?        \
>> >                     rte_constant_bswap32(x) :        \
>> >                     rte_arch_bswap32(x)))
>> >
>> > Have you tried CONFIG_RTE_FORCE_INTRINSICS=y ?
>> > It should trigger this definition:
>> >
>> > #define rte_bswap32(x) __builtin_bswap32(x)
>>
>> Yes.  CONFIG_RTE_FORCE_INTRINSICS=y works on Power Big endian.  But if
>> I turn off this, the error happens.  Seems rte_constant_bswap32
>> doesn't work on Power8? I'll try to check it.
>
>The tile arch force intrinsics:
>
http://dpdk.org/browse/dpdk/tree/config/defconfig_tile-tilegx-linuxapp-gcc
>#n39
>
http://dpdk.org/browse/dpdk/tree/lib/librte_eal/common/include/arch/tile
>/rte_byteorder.h#n36
>I don't know wether it was for the same reason.

Not really, the reason Tile arch force intrinsics is to reuse some generic
functions in
lib/librte_eal/common/include/generic/, such as
rte_atomic16_cmpset()
rte_atomic16_inc()
rte_spinlock_lock()
....

Thanks
-Zhigang