Fedora 20 / gcc 4.8.3 compilation is broken

[shamisp]

make[2]: Entering directory `/home/pasha/work/ucx/build-dev/test/perf'
  CC       libperf.lo
/home/pasha/work/ucx/contrib/../test/perf/libperf.c: In function 'uct_perf_test_setup_endpoints':
/home/pasha/work/ucx/contrib/../test/perf/libperf.c:305:14: warning: variable 'group_index' set but not used [-Wunused-but-set-variable]
     unsigned group_index, group_size, i;
              ^

Nothing wrong with the code. I think gcc 4.8.3 is broken.

Workaround for this:

diff --git a/configure.ac b/configure.ac
index be1f032..4f4d924 100644
--- a/configure.ac
+++ b/configure.ac
@@ -90,7 +90,7 @@ m4_ifdef([AS_VAR_APPEND],
 #
 # Initialize CFLAGS
 #
-CFLAGS="-g -Wall -Werror $UCX_CFLAGS"
+CFLAGS="-g -Wall $UCX_CFLAGS"
 CXXFLAGS="$CFLAGS"

So, I suggest remove -Werror . Any other ideas ?

[shamisp]

The nice way to do it is introduce GCC version check like here: http://stackoverflow.com/questions/7067385/find-the-gcc-version

For 4.8.3 we may remove the warning and keep it for all the rest

[tonycurtis]

Compile broke on our whale system with gcc 4.6.2 and 4.9.1 as well. Why not remove the variable/assignment and keep the stricter error checking?

tony

[shamisp]

The variable is actually used.

[tonycurtis]

On Nov 13, 2014, at 9:29 PM, Pasha [email protected] wrote:

The variable is actually used.

I can see it being assigned to (declared #305, assigned #343), but no other reference to that variable. Is this assignment affecting line #273 too?

tony

[shamisp]

As far as I remember, It is passed somewhere to the rte_call()

[tonycurtis]

On Nov 13, 2014, at 9:39 PM, Pasha <[email protected]mailto:[email protected]> wrote:

As far as I remember, It is passed somewhere to the rte_call()

Could we then just do the following to set the persistent group_index value?

diff --git a/test/perf/libperf.c b/test/perf/libperf.c
index 862fd68..b1e9287 100644
--- a/test/perf/libperf.c
+++ b/test/perf/libperf.c
@@ -302,7 +302,7 @@ static ucs_status_t ucx_perf_run_put_lat(uct_perf_context_t *perf)

ucs_status_t uct_perf_test_setup_endpoints(uct_perf_context_t *perf)
{

• unsigned group_index, group_size, i;
• unsigned group_size, i;
  uct_iface_addr_t *iface_addr;
  uct_ep_addr_t *ep_addr;
  uct_iface_attr_t iface_attr;
  @@ -340,7 +340,7 @@ ucs_status_t uct_perf_test_setup_endpoints(uct_perf_context_t *perf)

address = (uintptr_t)perf->super.recv_buffer;

• group_index = rte_call(&perf->super, group_index);
• (void) rte_call(&perf->super, group_index);
  group_size = rte_call(&perf->super, group_size);

perf->peers = calloc(group_size, sizeof(*perf->peers));

[shamisp]

I think you are actually right. group_index = rte_call(&perf->super, group_index); is not used anywhere and can be removed. Can you please submit pull request. Mellanox testing will pickup it automatically.

[tonycurtis]

On Nov 13, 2014, at 10:13 PM, Pasha <[email protected]mailto:[email protected]> wrote:

I think you are actually right.

Had to happen sooner or later :)

group_index = rte_call(&perf->super, group_index); is not used anywhere and can be removed. Can you please submit pull request. Mellanox testing will pickup it automatically.

I wonder if rte_call has a side-effect that is being picked up on line 273 but haven’t disentangled the pointer/struct chain yet -- calling the local variable “group_index” as well might be a bit of a red herring.

But there are subsequent compile issues so haven’t been able to verify (will do separate report to keep this one clean).

tony

[tonycurtis]

On Nov 13, 2014, at 10:28 PM, Curtis, Tony <[email protected]mailto:[email protected]> wrote:

group_index = rte_call(&perf->super, group_index); is not used anywhere and can be removed. Can you please submit pull request. Mellanox testing will pickup it automatically.

I wonder if rte_call has a side-effect that is being picked up on line 273 but haven’t disentangled the pointer/struct chain yet -- calling the local variable “group_index” as well might be a bit of a red herring.

test/perf/perftest.c makes me think the group_index() call is simply meant to be pure and return a value, in which case we can simply delete the variable and assignment.

I would like a second pair of more-experience-with-this-code eyes on it before doing anything…

tony

[yosefe]

yes, group_index is unused. my bad. need to be fixed. unless someone does it before, i will address it.