Opened 9 years ago
Closed 9 years ago
#50958 closed defect (fixed)
py-graph-tool fails to build with clang OpenMP option (configure.args=--enable-openmp)
Reported by: | essandess (Steve Smith) | Owned by: | count0 (Tiago de Paula Peixoto) |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | 2.3.4 |
Keywords: | Cc: | mamoll (Mark Moll), setack@…, petrrr | |
Port: | py-graph-tool |
Description
Macports clang 3.7 and 3.8 now support OpenMP, which creates the opportunity to build graph-tool on OS X with OpenMP speeds.
I expected that the option configure.args=--enable-openmp
would tell the autoconf'd ./configure
command to specify --enable-openmp
, but it does not:
# 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.8 +openmp sudo port -d install py-graph-tool py27-graph-tool configure.compiler=macports-clang-3.8 configure.args=--enable-openmp … DEBUG: Executing command line: cd "/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" && ./configure --prefix=/opt/local --with-boost=/opt/local --exec-prefix=/opt/local/Library/Frameworks/Python.framework/Versions/2.7 … checking whether to enable parallel algorithms with openmp... no
The port notes for libomp
say:
Use with clang-3.[78] (when built with +openmp variant) by adding "-fopenmp" to your compile and link lines. (Or "-I/opt/local/include/libomp -L/opt/local/lib/libomp -fopenmp" if clang is installed without +openmp.)
Based on the Port Phases page https://guide.macports.org/chunked/reference.phases.htm, I've tried a variety of options like configure.cxxflags-append=-fopenmp
, all without success.
I'd also ask for consideration whether adding OpenMP support should be the default behavior for the Macports graph-tool build now that it's available.
Attachments (2)
Change History (27)
comment:1 Changed 9 years ago by mamoll (Mark Moll)
comment:2 follow-up: 5 Changed 9 years ago by essandess (Steve Smith)
After applying this patch, clang-3.8 fails with the 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
I'll go try clang-3.7...
comment:3 Changed 9 years ago by mamoll (Mark Moll)
If compilation fails, you might want to file a ticket upstream at https://git.skewed.de/count0/graph-tool/issues/.
comment:4 Changed 9 years ago by essandess (Steve Smith)
On my system clang-3.7 fails with this error:
In file included from graph_betweenness.cc:20: In file included from /opt/local/include/boost/python.hpp:41: /opt/local/include/boost/python/make_constructor.hpp:177:32: warning: unused typedef 'assertion' [-Wunused-local-typedef] >::too_many_keywords assertion; ^ fatal error: error in backend: Cannot select: 0x7fe332ed22a0: f80 = zero_extend 0x7fe332ed1e00 [ORD=3] [ID=4] 0x7fe332ed1e00: i64 = FrameIndex<14> [ID=1] In function: .omp_outlined..24 clang: error: clang frontend command failed with exit code 70 (use -v to see invocation)
The build commands are:
sudo port install clang-3.7 +openmp sudo port -d install py-graph-tool configure.compiler=macports-clang-3.7
comment:5 follow-up: 6 Changed 9 years ago by count0 (Tiago de Paula Peixoto)
Replying to s.t.smith@…:
After applying this patch, clang-3.8 fails with the 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
This seems like a perfectly valid OpenMP annotation to me...
comment:6 follow-up: 7 Changed 9 years ago by essandess (Steve Smith)
Replying to tiago@…:
This seems like a perfectly valid OpenMP annotation to me...
Sounds like the upstream ticket should go to clang. I'll post one there.
comment:7 Changed 9 years ago by essandess (Steve Smith)
Replying to s.t.smith@…:
Replying to tiago@…:
This seems like a perfectly valid OpenMP annotation to me...
Sounds like the upstream ticket should go to clang. I'll post one there.
Oh wait -- there's a clang-3.9. I'll attempt compiling with the latest version.
comment:8 Changed 9 years ago by essandess (Steve Smith)
Still no luck with clang-3.9. Same error as above with 3.8:
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:
I'll post bug report to clang.
comment:9 Changed 9 years ago by essandess (Steve Smith)
Upstreamed to ticket https://trac.macports.org/ticket/50983
comment:10 follow-up: 14 Changed 9 years ago by essandess (Steve Smith)
The feedback from the upstream ticket mentions that omp's parallel if clause has a "parallel
"-named qualifier, which causes the compilation problem. https://trac.macports.org/ticket/50983#comment:8
I confirm that replacing the variable name "parallel
" with "parallel_enabled
" yields a successful compile. Patch files attached.
Replying to larryv@…:
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-blockwhere 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.
Changed 9 years ago by essandess (Steve Smith)
Attachment: | graph_blockmodel.hh.patch added |
---|
Patch file to replace variable name "parallel" with "parallel_enabled"
Changed 9 years ago by essandess (Steve Smith)
Attachment: | graph_blockmodel_overlap.hh.patch added |
---|
Patch file to replace variable name "parallel" with "parallel_enabled"
comment:11 Changed 9 years ago by essandess (Steve Smith)
Once Tiago agrees, this gets us back to the original problem: adding OpenMP support to the py-graph-tool
port.
Adding the "port
" command line argument "configure.args=--enable-openmp
" or "configure.args-append=--enable-openmp
" doesn't append "--enable-openmp
" to "./configure
".
comment:12 Changed 9 years ago by mf2k (Frank Schima)
Keywords: | graph-tool clang openmp libomp removed |
---|---|
Owner: | changed from macports-tickets@… to tiago@… |
comment:13 follow-up: 15 Changed 9 years ago by mamoll (Mark Moll)
Thanks for the patches. Something like this works for clang 3.[89]:
if {${configure.compiler} eq "macports-clang-3.8" || ${configure.compiler} eq "macports-clang-3.9"} { configure.args-append --enable-openmp }
but I can't distinguish between clang-3.7 and clang-3.7+openmp, so it seems safest to assume the clang-3.7 doesn't support OpenMP.
comment:14 Changed 9 years ago by count0 (Tiago de Paula Peixoto)
Replying to s.t.smith@…:
I confirm that replacing the variable name "
parallel
" with "parallel_enabled
" yields a successful compile. Patch files attached.
These patches seem fine. Please submit them as well to the upstream repository at: https://git.skewed.de/count0/graph-tool
comment:15 Changed 9 years ago by essandess (Steve Smith)
Replying to mmoll@…:
Thanks for the patches. Something like this works for clang 3.[89]:
if {${configure.compiler} eq "macports-clang-3.8" || ${configure.compiler} eq "macports-clang-3.9"} { configure.args-append --enable-openmp }but I can't distinguish between clang-3.7 and clang-3.7+openmp, so it seems safest to assume the clang-3.7 doesn't support OpenMP.
I'm testing with the latest upgrades and have run into the problem that clang_select
shows "mp-clang-3.9
", *not* "macports-clang-3.9
":
$ sudo port select --list clang Available versions for clang: mp-clang-3.9 (active) none $ sudo port select --set clang macports-clang-3.9 Selecting 'macports-clang-3.9' for 'clang' failed: The specified version 'macports-clang-3.9' is not valid. $ sudo port select --set clang mp-clang-3.9 Selecting 'mp-clang-3.9' for 'clang' succeeded. 'mp-clang-3.9' is now active.
The current port file doesn't detect that I've set clang-3.9 to be the default:
sudo port -d install py-graph-tool ... checking whether to enable parallel algorithms with openmp... no
I've also confirmed that if I edit the port file to say the following, --enable-openmp
is not passed to ./configure
:
if {${configure.compiler} eq "mp-clang-3.8" || ${configure.compiler} eq "mp-clang-3.9"} { configure.args-append --enable-openmp }
Then this:
sudo port -d install py-graph-tool ... checking whether to enable parallel algorithms with openmp... no
If I pass the command line argument configure.args-append=--enable-openmp
, the argument is not passed:
sudo port -d install py-graph-tool configure.args-append=--enable-openmp ... checking whether to enable parallel algorithms with openmp... no
If I change the port file back to its original, then the --enable-openmp
argument is passed with the command. This is the only thing that's working so far:
sudo port -d install py-graph-tool configure.compiler=macports-clang-3.9 ... DEBUG: Assembled command: 'cd "/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" && ./configure --prefix=/opt/local --with-boost=/opt/local --exec-prefix=/opt/local/Library/Frameworks/Python.framework/Versions/2.7 --enable-openmp' ... checking whether to enable parallel algorithms with openmp... yes
comment:16 Changed 9 years ago by essandess (Steve Smith)
Tiago's update from this ticket https://git.skewed.de/count0/graph-tool/issues/289 is already reflected in the mirrors, so all that's needed now is for the portfile to pass --enable-openmp
to ./configure
.
comment:17 Changed 9 years ago by mamoll (Mark Moll)
I don't think port select clang ...
changes the default compiler that MacPorts uses. It just creates symlinks so that clang points to clang-3.9. You can change the compiler by specifying it in ${prefix}/etc/macports/macports.conf. This shouldn't be necessary:
configure.args-append=--enable-openmp
It's already part of the Portfile.
comment:18 Changed 9 years ago by essandess (Steve Smith)
Is there a setting that will allow
sudo port upgrade outdated
or
sudo port install py-graph-tool
to automatically compile graph-tool upgrades with OpenMP support, without having to pass a configure.compiler=macports-clang-3.9
by hand?
comment:19 Changed 9 years ago by essandess (Steve Smith)
Something like the following tests whether mp-clang-3.9
is set to be the compiler, and avoids the issues of having to change macports.conf
whenever clang is upgraded.
port select --show clang | perl -ne "chomp; s/The currently selected version for 'clang' is '(.*)'./\$1/; print $_;"
comment:20 follow-up: 21 Changed 9 years ago by mamoll (Mark Moll)
We can add clang38/clang39 variants:
variant clang38 description "Use clang-3.8 and enable OpenMP" conflicts clang39 { configure.compiler macports-clang-3.8 } variant clang38 description "Use clang-3.9 and enable OpenMP" conflicts clang38 { configure.compiler macports-clang-3.9 }
The next question is then do you think +clang38 should be a default variant?
comment:21 follow-up: 23 Changed 9 years ago by essandess (Steve Smith)
Replying to mmoll@…:
We can add clang38/clang39 variants:
variant clang38 description "Use clang-3.8 and enable OpenMP" conflicts clang39 { configure.compiler macports-clang-3.8 } variant clang38 description "Use clang-3.9 and enable OpenMP" conflicts clang38 { configure.compiler macports-clang-3.9 }The next question is then do you think +clang38 should be a default variant?
Yes, variants are a great idea.
My personal preference would be default OpenMP support because multithreading makes my code scream on large graphs, and all my boxes have multiple cores anyway. I suspect this is common among graph-tool users, although I'd defer to Tiago or others who have experience with omp build robustness.
comment:22 Changed 9 years ago by essandess (Steve Smith)
PS typo in the clang39 variant if you go that route.
comment:23 Changed 9 years ago by count0 (Tiago de Paula Peixoto)
My personal preference would be default OpenMP support because multithreading makes my code scream on large graphs, and all my boxes have multiple cores anyway. I suspect this is common among graph-tool users, although I'd defer to Tiago or others who have experience with omp build robustness.
I always enable openmp with GCC, and it is the default on all the packages I maintain (Debian/Ubuntu and Arch). Assuming the clang implementation is stable (I have no idea), it should be the default.
comment:24 Changed 9 years ago by essandess (Steve Smith)
Rather than a "clang39" variant, I'd suggest an "openmp" variant that would specify whatever the latest clang version is and pass configure.args-append=--enable-openmp
. And make this variant the default as Tiago says.
comment:25 Changed 9 years ago by mamoll (Mark Moll)
Resolution: | → fixed |
---|---|
Status: | new → closed |
Fixed in r147229.
Can you change the Portfile like so:
That seems to work for me. On my machine there is no +openmp variant for clang-3.8 (it's included by default). The clang-3.7 *does* has an +openmp variant. The version of clang that comes with the latest Xcode doesn't support openmp at all. How do I do the right thing in all cases?