Make the CommsUtils functions inline #21
Conversation
I tried to compile the current QPhiX devel branch (b1dbed4) together with the current Chroma devel branch (62cf4d341f5e4811637d37e54a9345598faea848). This has failed due to multiple definition errors like this: ../../lib/libchroma.a(syssolver_mdagm_aggregate.o): In function `QPhiX::CommsUtils::sumDouble(double*)': syssolver_mdagm_aggregate.cc:(.text+0x180): multiple definition of `QPhiX::CommsUtils::sumDouble(double*)' ../../lib/libchroma.a(syssolver_linop_aggregate.o):syssolver_linop_aggregate.cc:(.text+0x1d0): first defined here Drilling down into QPhiX I noticed that the file `include/comm.h` is the only source of a definition for this function. There are three preprocessor branches that are in this file. I have added a sentinel in the previous commit such that one cannot mess it up. The culprit seems to be that the function definitions are not inline. Having a member function defined within the `class` automatically makes it inline. Those are no problem in the header. The functions in question however are only free functions in a namespace. Their definition in the header does not automatically make them inline, yielding to a multiple definition error as seen. I opted for adding `inline` as those functions just delegate to QDP++ if they do anything at all. The other way would be to split off the definition form the declaration into a `.cc` file.
|
Hi Martin,
You need the following:
i) QDP++ devel branch
ii) CHroma devel branch
iii) QPhiX devel branch.
The master branch of QPhiX in particular is very antiquated.
However, please note that QPhiX can only function as a ‘native’ dslash (instead of say cpp_wilson_dslash)
if QDP-JIT/LLVM is used underneath chroma. This is because QPhIX and QDP++/Chroma use different data layouts
by default. QDP-JIT/LLVM allows the QDP index order to be changed to match QPhiX, but the regular Chroma does
not allow this. So if you are trying to build chroma over regular QDP++ you should not enable the —enable-qphix-dslash
options to Chroma. If you use QPhiX with Chroma over regular QDP++ you should still be able to use the QPhiX solvers,
although for Twisted Mass you may need a solver wrapper to add to Chroma (the same way Clover is currently wrapped).
To answer your other previous question, yes, QPhiX originally started off from cpp_wilson_dslash long long ago, as that
was a code I had confidence in, but really it is quite different now.
With very best wishes,
Balint
… On Dec 1, 2016, at 6:17 AM, Martin Ueding ***@***.***> wrote:
I tried to compile the current QPhiX devel branch
(b1dbed4) together with the current
Chroma devel branch (62cf4d341f5e4811637d37e54a9345598faea848). This has
failed due to multiple definition errors like this:
../../lib/libchroma.a(syssolver_mdagm_aggregate.o): In function QPhiX::CommsUtils::sumDouble(double*)': syssolver_mdagm_aggregate.cc:(.text+0x180): multiple definition ofQPhiX::CommsUtils::sumDouble(double*)'
../../lib/libchroma.a(syssolver_linop_aggregate.o):syssolver_linop_aggregate.cc:(.text+0x1d0): first defined here
Drilling down into QPhiX I noticed that the file include/comm.h is the
only source of a definition for this function. There are three
preprocessor branches that are in this file. I have added a sentinel in
the previous commit such that one cannot mess it up.
The culprit seems to be that the function definitions are not inline.
Having a member function defined within the class automatically makes
it inline. Those are no problem in the header. The functions in question
however are only free functions in a namespace. Their definition in the
header does not automatically make them inline, yielding to a multiple
definition error as seen.
I opted for adding inline as those functions just delegate to QDP++ if
they do anything at all. The other way would be to split off the
definition form the declaration into a .cc file.
You can view, comment on, or merge this pull request online at:
#21
Commit Summary
• Add a sanity check for QPHIX_FAKE_COMMS and QPHIX_QMP_COMMS
• Make the CommsUtils functions inline
File Changes
• M include/qphix/comm.h (25)
Patch Links:
• https://github.com/JeffersonLab/qphix/pull/21.patch
• https://github.com/JeffersonLab/qphix/pull/21.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
--------------------------------------------------------------------------------------
Dr Balint Joo High Performance Computational Scientist
Jefferson Lab
12000 Jefferson Ave, Suite 3, MS 12B2, Room F217,
Newport News, VA 23606, USA
Tel: +1-757-269-5339, Fax: +1-757-269-5427
email: bjoo@jlab.org
-------------------------------------------------------------------------------------
|
|
Would it make sense to rename the I can currently not test whether switching QDP++ from My mental picture of all this software is still a bit blurry. Do I understand it correctly that QPhiX provides Dslash and solvers and that I can choose the Dslash from QPhiX or |
|
The issue still persists with the Can you get this build correctly without inlining the functions? |
|
I currently try to get Chroma running on the Marconi (Intel KNL) cluster in Italy. There I have the exact same linker error in I truly do not understand how it compiles on your machines. Since those functions are non-member non-template functions implemented in the header, there must be linker errors. This also happens with GCC and Intel C++ on three different machines now. Could this please be included in the canonical |
I have forgotten to add the header for the exception. It has built on my machine with GCC 6.3 but failed when testing with GCC 4.9 on Travis CI. This adds the missing header.
I tried to compile the current QPhiX devel branch
(b1dbed4) together with the current
Chroma devel branch (62cf4d341f5e4811637d37e54a9345598faea848). This has
failed due to multiple definition errors like this:
Drilling down into QPhiX I noticed that the file
include/comm.his theonly source of a definition for this function. There are three
preprocessor branches that are in this file. I have added a sentinel in
the previous commit such that one cannot mess it up.
The culprit seems to be that the function definitions are not inline.
Having a member function defined within the
classautomatically makesit inline. Those are no problem in the header. The functions in question
however are only free functions in a namespace. Their definition in the
header does not automatically make them inline, yielding to a multiple
definition error as seen.
I opted for adding
inlineas those functions just delegate to QDP++ ifthey do anything at all. The other way would be to split off the
definition form the declaration into a
.ccfile.