Re: [dpdk-dev] DPDK compilation on arm is failing in Travis

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

From: Michael Santana Francisco <msantana@redhat.com>
Sent: Wednesday, June 5, 2019 5:39 PM
To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Aaron Conole <aconole@redhat.com>; thomas@monjalon.net
Cc: Ruifeng Wang (Arm Technology China) <Ruifeng.Wang@arm.com>; Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com>; Dharmik Thakkar <Dharmik.Thakkar@arm.com>; jerin.jacob@caviumnetworks.com; yskoh@mellanox.com; dev@dpdk.org; bruce.richardson@intel.com; nd <nd@arm.com>
Subject: Re: DPDK compilation on arm is failing in Travis

On 6/5/19 5:36 PM, Honnappa Nagarahalli wrote:



Thomas Monjalon <thomas@monjalon.net><mailto:thomas@monjalon.net> writes:



05/06/2019 21:40, Aaron Conole:

Thomas Monjalon <thomas@monjalon.net><mailto:thomas@monjalon.net> writes:



The compilation of the master branch is failing for aarch64:

 https://travis-ci.com/DPDK/dpdk

The log is so much verbose that I am not able to understand what

is really wrong.

Please help to diagnose and fix, thanks.



A discussion about this:



http://mails.dpdk.org/archives/dev/2019-June/134012.html



I see the error now.

It is printing the full log after the error, so I missed the error

at the top.



I've read your comment about a possible error with the patch

removing weak functions but neither me nor Bruce were able to reproduce

it.

What is the condition to see this compiler warning?



It is only on ARM, and only when the neon intrinsics are in use.

I am not able to reproduce it from the tip of master.



I am using:

gcc (Ubuntu 8.3.0-6ubuntu1~18.04) 8.3.0



From the log on Travis, looks like the compiler is:

gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609



Is this the issue?



Why are we seeing the error now?

I tested with gcc-5 (Ubuntu/Linaro 5.5.0-12ubuntu1) 5.5.0 20171010, it works fine. I cannot get hold of 5.4.0. Not sure if needs to be supported.

Are there any issues in upgrading to 7 or 8?
I have tested it on my ubuntu 16.04 vm on commit 8cb511bb94ad92a76990f175cac76bb13d51daba (head of master seems to be failing for other reasons on my vm).
I tested the following gcc versions:

gcc 5.5.0 "cc (Ubuntu 5.5.0-12ubuntu1~16.04) 5.5.0 20171010"
gcc 7.4.0 "cc (Ubuntu 7.4.0-1ubuntu1~16.04~ppa1) 7.4.0"
gcc 8.1.0 "cc (Ubuntu 8.1.0-5ubuntu1~16.04) 8.1.0"

All tested versions failed on the exact same error shown in travis. I don't know if the compiler is at fault here. Maybe Aaron's patch is a viable option?

The issue is the vector lane setting code looks like:



   lval = lane_set(scalar, rval, lane id)



In this case, 'rval' is being used before it is ever set, but it

really could be just 0 for the first lane setting code.  Thereafter,

we use the old value of input as the rval, but each time a different lane is set.



It would be nice if there were an intrinsic that formatted correctly

from the start (something we could call like lval =

lane_set_from_array(scalar_array)).

[Honnappa] This exists already. ‘vdupq_n_s32’ can be used. Can you try the following?



honnag01@qc2400f-1:~/dpdk$ git diff

diff --git a/lib/librte_acl/acl_run_neon.h b/lib/librte_acl/acl_run_neon.h

index 01b9766d8..b3196cd12 100644

--- a/lib/librte_acl/acl_run_neon.h

+++ b/lib/librte_acl/acl_run_neon.h

@@ -181,8 +181,8 @@ search_neon_8(const struct rte_acl_ctx *ctx, const uint8_t **data,



        while (flows.started > 0) {

                /* Gather 4 bytes of input data for each stream. */

-               input0 = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 0), input0, 0);

-               input1 = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 4), input1, 0);

+               input0 = vdupq_n_s32(GET_NEXT_4BYTES(parms, 0));

+               input1 = vdupq_n_s32(GET_NEXT_4BYTES(parms, 4));



                input0 = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 1), input0, 1);

                input1 = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 5), input1, 1);

@@ -242,7 +242,7 @@ search_neon_4(const struct rte_acl_ctx *ctx, const uint8_t **data,



        while (flows.started > 0) {

                /* Gather 4 bytes of input data for each stream. */

-               input = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 0), input, 0);

+               input = vdupq_n_s32(GET_NEXT_4BYTES(parms, 0));

                input = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 1), input, 1);

                input = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 2), input, 2);

                                                input = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 3), input, 3);



Then 'input' would never appear as an rval before it was set.



I thought Jerin Jacob (CC'd) would have some opinion on the right fix.

There are three 'fixes' I know exist - one is to squelch the warning

(but I don't like it because it could hide future code that introduces

this), one is to create a static and use assignment, one is to replace

the first call and pass in a 0'd lane for the first one.



Actually, I think I have a patch that could work to not introduce an

assignment, but squelch the warning.  Something like the following (not

tested).



---



diff --git a/lib/librte_acl/acl_run_neon.h

b/lib/librte_acl/acl_run_neon.h index 01b9766d8..37c984fef 100644

--- a/lib/librte_acl/acl_run_neon.h

+++ b/lib/librte_acl/acl_run_neon.h

@@ -165,6 +165,7 @@ search_neon_8(const struct rte_acl_ctx *ctx, const

uint8_t **data,

    uint64_t index_array[8];

    struct completion cmplt[8];

    struct parms parms[8];

+   static int32x4_t ZEROVAL;

    int32x4_t input0, input1;



    acl_set_flow(&flows, cmplt, RTE_DIM(cmplt), data, results, @@ -

181,8 +182,8 @@ search_neon_8(const struct rte_acl_ctx *ctx, const

uint8_t **data,



    while (flows.started > 0) {

            /* Gather 4 bytes of input data for each stream. */

-           input0 = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 0), input0,

0);

-           input1 = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 4), input1,

0);

+           input0 = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 0),

ZEROVAL, 0);

+           input1 = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 4),

ZEROVAL, 0);



            input0 = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 1), input0,

1);

            input1 = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 5), input1,

1); @@

-227,6 +228,7 @@ search_neon_4(const struct rte_acl_ctx *ctx, const

uint8_t **data,

    uint64_t index_array[4];

    struct completion cmplt[4];

    struct parms parms[4];

+   static int32x4_t ZEROVAL;

    int32x4_t input;



    acl_set_flow(&flows, cmplt, RTE_DIM(cmplt), data, results, @@ -

242,7 +244,7 @@ search_neon_4(const struct rte_acl_ctx *ctx, const

uint8_t **data,



    while (flows.started > 0) {

            /* Gather 4 bytes of input data for each stream. */

-           input = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 0), input, 0);

+           input = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 0),

ZEROVAL, 0);

            input = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 1), input, 1);

            input = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 2), input, 2);

            input = vsetq_lane_s32(GET_NEXT_4BYTES(parms, 3), input, 3);

--

2.21.0