clang fails with valid OpenMP pragma on py-graph-tool build

[essandess (Steve Smith)]

The clang compile fails with an error on graph-tool with OpenMP enabled. The author believes that the OpenMP pragma is valid (I concur).

Here's how to tickle the bug:

# uninstall existing graph-tool
sudo port uninstall py-graph-tool py27-graph-tool
sudo port clean --dist py-graph-tool py27-graph-tool

# build new graph-tool with OpenMP support
sudo port install clang-3.9 libomp
# modify the graph-tool Portfile to --enable-openmp
sudo vi `port file py-graph-tool`
<    #configure.args-append --with-boost=${prefix} --exec-prefix=${python.prefix}
>    configure.args-append --with-boost=${prefix} --exec-prefix=${python.prefix} --enable-openmp
# compile graph-tool
sudo port -d install configure.compiler=macports-clang-3.9

clang error:

In file included from graph_blockmodel.cc:39:
In file included from ./graph_blockmodel_overlap.hh:24:
./graph_blockmodel.hh:2708:43: warning: missing ':' after directive name modifier - ignoring [-Wignored-pragmas]
            schedule(runtime) if (parallel)
                                          ^
./graph_blockmodel.hh:2708:43: error: expected expression
In file included from graph_blockmodel.cc:39:
./graph_blockmodel_overlap.hh:1337:67: warning: missing ':' after directive name modifier - ignoring [-Wignored-pragmas]
            firstprivate(m_entries) schedule(runtime) if (parallel)
                                                                  ^
./graph_blockmodel_overlap.hh:1337:67: error: expected expression
In file included from graph_blockmodel.cc:39:
In file included from ./graph_blockmodel_overlap.hh:24:
In file included from ./graph_blockmodel.hh:26:

The code is in /opt/local//var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_python_py-graph-tool/py27-graph-tool/work/graph-tool-2.13/src/graph/community/graph_blockmodel_overlap.hh:

        #pragma omp parallel for default(shared) private(i) \
            firstprivate(m_entries) schedule(runtime) if (parallel)
        for (i = 0; i < N; ++i)
        {
...

This appears to be correct, and compiles correctly with gcc on Linux.

[essandess (Steve Smith)]

Upstream of ticket ​https://trac.macports.org/ticket/50958

[essandess (Steve Smith)]

FWIW, the code compiles if the omp parallel clause "if (parallel)" is removed. This along with the error message "error: expected expression" suggests that clang is incorrectly interpreting the omp pragma "if" clause as the opening of a C++ "if" conditional.

[mamoll (Mark Moll)]

Can you submit a patch for py-graph-tool that (a) enables OpenMP when the compiler supports it and (b) removes all occurrences of "if (parallel)"?

[count0 (Tiago de Paula Peixoto)]

Replying to mmoll@…:

Can you submit a patch for py-graph-tool that (a) enables OpenMP when the compiler supports it and (b) removes all occurrences of "if (parallel)"?

Please don't make a patch to (b). The occurrences of "if (parallel)" are not superfluous, and if removed will break the code.

[mamoll (Mark Moll)]

Thanks for the explanation. I thought it was a compiler suggestion / optimization that wouldn't change the input/output behavior of the code. In that case, enabling openmp might not be that simple for clang at the moment.

[essandess (Steve Smith)]

Replying to tiago@…:

Please don't make a patch to (b). The occurrences of "if (parallel)" are not superfluous, and if removed will break the code.

Only if you call gt.blockmodel()!—other than that it's blindingly fast on an MP :-j

It looks like clang's interpreter needs fixing for libomp code. Hopefully this won't be a big deal.

[larryv (Lawrence Velázquez)]

​Clang supports directive name modifiers in OpenMP 4.5 if clauses. From §2.12 of the ​OpenMP 4.5 spec:

The syntax of the if clause is as follows:

if([directive-name-modifier :]scalar-expression)

And from §2.5:

The syntax of the parallel construct is as follows:

#pragma omp parallel [clause[[,]clause]...]new-line
structured-block

where clause is one of the following:

if([parallel :]scalar-expression)

…

This seems to make “parallel” a sort of reserved word that requires a trailing colon, so I doubt LLVM upstream would consider this a bug, although you’re free to ​submit a report. I suggest that graph-tool simply use any other variable name.

[essandess (Steve Smith)]

Replying to larryv@…:

I suggest that graph-tool simply use any other variable name.

Nice to be shot down in (BNF) style!

I confirm that replacing the variable name "parallel" with "parallel_enabled" yields a successful compile. (I resisted the temptation to call this "any_other_variable_name".)

I'll post the patch files to the downstream ticket, ​https://trac.macports.org/ticket/50958

[larryv (Lawrence Velázquez)]

Replying to s.t.smith@…:

(I resisted the temptation to call this "any_other_variable_name".)

:P

Closing as invalid since there’s no real problem with Clang or our port.