Support Windows using MSVC #5

GreenGary · 2020-03-30T13:01:35Z

Based on preparation made in isse #4 , the library shall be usable on Windows using MSVC toolchain. Target would be MSVC++ 14.1 _MSC_VER == 1910 (Visual Studio 2017 version 15.0; support for Visual Studio < 2015 would require more refactorintg, see this Microsoft document.

Todos:

fix compile errors (not that many)
align compiler settings
extend cmake script to support MSVC
if using dll, all static variables have to be checked thoroughly for portability

Remarks:

Linux build creates a shared library (so), the equivalent for Windows would be a dll;
- Because of the extensive use of templates, the dll might be anemic
- Because of Microsoft's weird name mangling, a linker lib would be part of the package in addition to dll and headers
- Maybe it would simplify the usage, if just a static lib + header would make the interface?
- a static lib would avoid the need for any means (likely a macro) to inject " __declspec(dllexport)" for functions exported by the Windows dll.

=> issue to be discussed!

GreenGary · 2020-03-30T17:17:24Z

@rolandzander FYI

friedrichatgc · 2020-03-30T17:18:55Z

At least in combination of fmatvec with MBSim a so/dll of fmatvec is required: fmatvec uses boost::spirit for parsing vectors/matrices as well as symbolic expressions. Since vector/matrix/expressions are all templated we explicitly instantiate MANY common template variants in the so/dll. If not doing so all the boost::spirit code must be compiled in every program using fmatvec (MBSim). This increases the compile time of MBSim by factors.

Handling of the lib file should work out of the box in cmake.

All the declspec things for windows may be problematic for a dll, but I think its just work to do.

GreenGary · 2020-04-13T16:02:37Z

@friedrichatgc I have found another compilation issue, I am not sure how to address. The friend declarations in class Operation are not considered by the MSVC compiler. Error is fmatvec\ast.cc(645): cannot access private member declared in class 'fmatvec::AST::Operation'

Fixed: Missing namespace confused MSVC - will drop a pull request to fix this.

Initialized once and never modified.

MSVC requires some location in the memory for SymbolicExpression::constructSymbol to compile.

…ation

…plex.h MSVC does not consider the operators from linear_algebra_complex.h if they are declared after usage, e.g. in Vector operator*(&x, &alpha) {}

GreenGary · 2020-04-18T13:32:07Z

@friedrichatgc I would like to know your opinion about the dllexport/dllimport issues, when using MSVC. I played around with cmake / msvc but cannot make a final decision on that.

gcc linux build exports about 2.000 symbols, mingw about 1.500 in total (global, weak, undefined, text, code, ...)

The most general approach to export symbols is using set_target_properties( fmatvec PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS 1 ) for the dll. This exports ~17.000 symbols and increases dll/lib's size considerable. In addition, it does not fix dllexport/dllimport issues with static variables defined in the dll (e.g. static unsigned long evalOperationsCount;)
Prefix all exported symbols with a macro and set macro to __declspec(dllexport) or __declspec(dllexport) (or empty for gcc).
Maintain exports in a separate def file
Call cmake -E __create_def only on some of the obj files to generate the def
Create the def file on all/some obj files and filter the output.

My thoughts about potential solutions:

Will cause issues in real life due to duplicate symbols etc. ; increase in size is not acceptable.
Would be the most declarative way, but it infringes the complete code base - and it is just clutter for gcc build.
Maintaining the def file manually will break MSVC build each and every time - especially with all the templated stuff, it will become a nightmare to maintain; same issue with static variables as in 1.
Might be a good approach, if a subset can be defined. I do not know which obj would be the right subset - I tried linear_algebra_complex.cc.obj linear_algebra_double.cc.obj which returned a reasonable list of symbols. Same issue with static variables as in 1.
May always be a little fuzzy, but maybe a good compromise (if 2. is not an option). Same issue with static variables as in 1.

GreenGary · 2020-04-20T06:40:35Z

I created a prototype in my fmatvec-fork with a combination of 4. and 5. : https://github.com/GreenGary/fmatvec/tree/2020-04-20-cmake-msvc

…ebra_complex.h" This reverts commit 36059bb. Add template function declarations for complex types to linear_algebra.h

friedrichatgc · 2020-04-20T16:57:29Z

I think the best and most common way is option 2:

using "__declspec(dllexport)" for MSVS
using "attribute ((visibility ("default")))" for gcc while compiling everything with "-fvisibility=hidden"

This is also the "todays" usual way for gcc and does not clutter gcc. But sure it requires a lot of changes in the code.
I don't like any option which decouples the code (.h file) and the export definition (.def file). Both should be in the same file -> use __declspec/attribute.

GreenGary · 2020-04-30T13:07:51Z

@friedrichatgc I am currently setting up the required dll exports using __declspec. I noticed, that most matrixes have prepared shift operators, but not DiagMat . Any matrix except DiagMat can be linked without boost::spirit::qi::rule being exported. Is this missing from stream.h and stream.cc like below? And why does this not compile?

extern template std::istream MSVC_DLL_DECLSPEC & operator>>(std::istream &s,       Matrix<Diagonal, Ref,     Ref,     double              > &A);
extern template std::ostream MSVC_DLL_DECLSPEC & operator<<(std::ostream &s, const Matrix<Diagonal, Ref,     Ref,     double              > &A);

Found while trying to build testfunction.cc: { istringstream str(inputd); DiagMat m; str>>m; cout<<m<<endl; }

friedrichatgc · 2020-04-30T18:51:18Z

Building testfunction.cc should work without DiagMat explicitly noted in stream.h|.cc since testfunction.cc include stream_impl.h. So I don't know what is wrong here but we can also add DiagMat to stream.h|.cc. I have just pushed this (maybe you forgot to include diagonal_matrix.h).
If this help fine but I have no idea why boost::spirit::qt::rule needs to exported? It from a boost header only lib.

GreenGary · 2020-05-11T18:20:07Z

I have a draft for the library exports using declspec / attribute ((visibility ("default"))) :
https://github.com/GreenGary/fmatvec/tree/control-exported-symbols
fmatvec and its checks build successfully. Library now exports 660 symbols less than on the master branch (2214). I tried to export only reasonable stuff, maybe some symbols are missing now. However, I do not get linker errors when running mbsim build script. However, the script raises an error (which as far as I understand, is no related to linkage):

/mbsim-env/mbsim/kernel/mbsim/element.h:56: Error: 'fmatvec::Atom' is not a valid base class.
/mbsim-env/local-docker/include/fmatvec/atom.h:115: Error: See definition of 'fmatvec::Atom'.
/mbsim-env/mbsim/kernel/mbsim/element.h:56: Warning 401: Nothing known about base class 'fmatvec::Atom'. Ignored.

Since I do not know anything about SWIG, I can just guess, this error comes from the additional macro?

friedrichatgc · 2020-05-12T17:48:02Z

I have added this branch to mbsim-env/fmatvec and enabled this branch for CI, see https://www.mbsim-env.de/mbsim/linux64-ci/report/result_0000000409/index.html
I will fix the SWIG problem next.

friedrichatgc · 2020-05-13T19:53:48Z

mbsim/master is now updated to work with fmatvec/control-exported-symbols, see
https://www.mbsim-env.de/mbsim/linux64-ci/report/result_0000000413/index.html

friedrichatgc · 2020-07-16T17:25:07Z

The staging build system at wwwstaging.mbsim-env.de (only accessible via IPv6) is now ready to build mbsim tools using cmake/ninja.
The staging system is configured to use the fmatvec cmake branch as default instead of the master branch.
If a CMakeLists.txt file is found in the basedir then cmake/ninja is used, if not autotools are used for building.
The requirements of cmake/ninja based tools are that the following targets exists:

the default target = all = "none specified": build the actual tool
clean: remove all files generated by all
install: install the tool to CMAKE_INSTALL_PREFIX
check: build and run "unit" tests of the tool
doc/all (only if a subdir named doc exists): build doxygen documentation
doc/clean (only if a subdir named doc exists): clean the files generated by doc/all
doc/install (only if a subdir named doc exists): install the doxygen documentation
xmldoc/all (only if a subdir named xmldoc exists): build xml documentation
xmldoc/clean (only if a subdir named xmldoc exists): clean the files generated by xmldoc/all
xmldoc/install (only if a subdir named xmldoc exists): install the xml documentation

The first three are default cmake targets. The check target is also already implemented on the cmake branch. The doc/* targets are missing. This is also the reason for all failures of the doc build of all other tools. xmldoc/* is not needed for fmatvec.

All failures seen on the staging build system seem to be related to missing parts of the cmake/ninja build of fmatvec. But now we have a working testing environment for the complete project using the cmake/ninja fmatvec variant.

rolandzander · 2020-07-17T05:55:25Z

I will try to address the targets "doc/..." within the next two weeks. Are there other open issues to be resolved before the cmake build can fully be replacement when building fmatvec? In case of fmatvec, I see no conflict in merging branch "cmake" to "master". Shall we plan to extract the automake parts to a separate branch and make "master" provide cmake including native Win build? Shall both cmake and automake be continuously maintained. Am 16.07.20 um 19:25 schrieb Markus Friedrich:

…

The staging build system at wwwstaging.mbsim-env.de (only accessible via IPv6) is now ready to build mbsim tools using cmake/ninja. The staging system is configured to use the fmatvec cmake branch as default instead of the master branch. If a CMakeLists.txt file is found in the basedir then cmake/ninja is used, if not autotools are used for building. The requirements of cmake/ninja based tools are that the following targets exists: * the default target = all = "none specified": build the actual tool * clean: remove all files generated by all * install: install the tool to CMAKE_INSTALL_PREFIX * check: build and run "unit" tests of the tool * doc/all (only if a subdir named doc exists): build doxygen documentation * doc/clean (only if a subdir named doc exists): clean the files generated by doc/all * doc/install (only if a subdir named doc exists): install the doxygen documentation * xmldoc/all (only if a subdir named xmldoc exists): build xml documentation * xmldoc/clean (only if a subdir named xmldoc exists): clean the files generated by xmldoc/all * xmldoc/install (only if a subdir named xmldoc exists): install the xml documentation The first three are default cmake targets. The check target is also already implemented on the cmake branch. The doc/* targets are missing. This is also the reason for all failures of the doc build of all other tools. xmldoc/* is not needed for fmatvec. All failures seen on the staging build system seem to be related to missing parts of the cmake/ninja build of fmatvec. But now we have a working testing environment for the complete project using the cmake/ninja fmatvec variant. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADHK2DG4HANVZSMGPHZQR33R34ZYHANCNFSM4LWSC5KA>.

friedrichatgc · 2020-07-17T17:52:35Z

Just all issues on wwwstaging.mbsim-env.de must be resolved before merging.
The "make" error in "hdf5serie/h5plotserie" seems also to be related to fmatvec: fmatvec seem not to put all its dependencies to fmatvec.pc (at least the boost include dir seem to be missing).
The Windows build of fmatvec is also not working: seems to be a missing blas lib when linking (on Linux linking against lapack will implicitly link with blas but on Windows such implicit linking does not take place, as far as I know).
More errors may be seen if the current ones are fixed.

friedrichatgc · 2020-07-17T21:42:33Z

I would remove the autotools part from the cmake branch and if all works on wwwstaging merge it to master. Before merging fmatvec the staging branch of the build repo must be merged to master to enable cmake on www.mbsim-env.de.
Supporting both cmake and autotools is no option for me.

GreenGary · 2020-07-22T05:57:18Z

The Windows build of fmatvec is also not working: seems to be a missing blas lib when linking (on Linux linking against lapack will implicitly link with blas but on Windows such implicit linking does not take place, as far as I know).

I successfully used LAPACK/BLAS with x86 builds from http://icl.cs.utk.edu/lapack-for-windows/lapack/LAPACKE_examples.zip - these were the only MSVC builds (not mingw) I found. Without an Intel Fortran on hands, I could not build myself. Roland links against IntelMKL.
So my question is: Which BLAS/LAPACK is in use on the CI system?

friedrichatgc · 2020-07-22T17:44:53Z

The CI system builds lapack/blas from source using mingw-cross-compiler, see https://github.com/mbsim-env/build/blob/master/docker/buildwin64Image/Dockerfile
Since this build is not found by fmatvec-cmake automatically the system provides to cmake the variables BLAS_LIBRARIES, LAPACK_LIBRARIES, BLAS and LAPACK, see https://github.com/mbsim-env/build/blob/staging/docker/buildwin64Image/entrypoint.py
But cmake seems just to past the lapack liberary to the linker command but not blas, see https://wwwstaging.mbsim-env.de/base/textFieldFromDB/builds/Tool/1309/makeOutput/

rolandzander · 2020-07-28T08:28:34Z

I try to fix the windows build testing and accouting for BLAS. The boost-include path thing to me seems to be a problem of fmatvec AND h5plotserie: all other packages depending on fmatvec succeed in their build, and h5plotserie can not resolve include of boost/filesystem.h which is not required by fmatvec. Unfortunately I do also not find include directives in the fmatvec.pc of the autmake build+install, so no valid source for me to copy from! I now add include directive to cmake-generated fmatvec.pc since fmatvec definetly carries that dependency - still h5plotseries seems unclear to me.

friedrichatgc · 2020-07-28T17:55:57Z

Note that wwwstaging.mbsim-env.de is currently not running since everything is merged now to www.mbsim-env.de which does not CI also for the fmatvec cmake branch and also does the nightly builds (including windows) for the fmatvec cmake branch. See "Continuous Integration (CI) Linux64 Debug Build" and "Daily builds of none default branches" on www.mbsim-env.de

You will see tomorrow the results of your changes their (these builds run between about 6:00 and 8:30).

The boost include thing I see as a pure fmatvec problem since h5plotserie just correctly relies on the implicit dependency to boost which is brought in by fmatvec (all other tools just have a explicit dependency to boost either due to additional binary boost library requirements or due to historical reasons which fmatvec does not require boost at all).
Adding the proper boost include directive to fmatvec.pc should resolve the issue as it is done by the autotools generated fmatvec.pc (the @define@ variable pushes the boost include their).

rolandzander · 2020-07-30T08:19:54Z

For the "doc" and "xmldoc" parts we might need different target names (for all/clean/insall), since cmake does not support the definition of target names with (forward) slashes, see https://gitlab.kitware.com/cmake/cmake/-/issues/20774. Kitware suggests to implement targets like "doc-all/doc-install/doc-clean" instead.
Does this collide with the current build definition? Automake allows for target grouping via directories, I don't see a comparable mechanism for cmake?!?

friedrichatgc · 2020-07-30T20:36:07Z

I added it to the build system as target doc/all due to this issue https://gitlab.kitware.com/cmake/cmake/-/issues/16677
This seems to be in contrast to the issue you found.
But I can change the target name the build system uses for cmake/ninja to doc-all, doc-install, ... or maybe better doc, doc-install, ...?

rolandzander · 2020-07-30T20:49:12Z

The drawback of adding results of target doc to install via cmake's install(DIRECTORY html ...) is that doc will be added to top level install target, which is in contrast to current behavior and would completely mix up current separation. Also I did not see a simple way to introduce the dependency of install on target doc in that case.
cmake's mechanism for separation of installation parts is the COMPONENT specification in command install(...) as used in current commit.
If changing target names, and these will be common for GNU-make and ninja when not using slashes, I do not insist on any naming convention - feel free to suggest or change as you like (as you can see, doc-all is only alias for doc which I preferred when implementing).

friedrichatgc · 2020-07-30T20:57:30Z

I don't understand your comment but will change the targets to doc, doc-clean, doc-install.

GreenGary mentioned this issue Mar 31, 2020

MSVC compile error C3066: there are multiple ways ... and "and" #6

Closed

GreenGary pushed a commit that referenced this issue Apr 13, 2020

#5 Add constness to Operation::opMap

5f03af1

Initialized once and never modified.

GreenGary pushed a commit that referenced this issue Apr 13, 2020

#5 Add explicit initialization to SymbolicExpression::constructSymbol

30df7bf

MSVC requires some location in the memory for SymbolicExpression::constructSymbol to compile.

GreenGary pushed a commit that referenced this issue Apr 13, 2020

#5 MSVC requires the correct namespace to recognize the friend declar…

24016cf

…ation

GreenGary pushed a commit that referenced this issue Apr 18, 2020

#5 remove duplicate include

b611c4b

GreenGary self-assigned this Apr 18, 2020

friedrichatgc pushed a commit that referenced this issue Apr 20, 2020

Revert "#5 fix msvc compile errors ignoring operators from linear_alg…

48269cd

…ebra_complex.h" This reverts commit 36059bb. Add template function declarations for complex types to linear_algebra.h

friedrichatgc closed this as completed Apr 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Windows using MSVC #5

Support Windows using MSVC #5

GreenGary commented Mar 30, 2020 •

edited

Loading

GreenGary commented Mar 30, 2020

friedrichatgc commented Mar 30, 2020

GreenGary commented Apr 13, 2020 •

edited

Loading

GreenGary commented Apr 18, 2020

GreenGary commented Apr 20, 2020

friedrichatgc commented Apr 20, 2020

GreenGary commented Apr 30, 2020

friedrichatgc commented Apr 30, 2020

GreenGary commented May 11, 2020

friedrichatgc commented May 12, 2020

friedrichatgc commented May 13, 2020

friedrichatgc commented Jul 16, 2020

rolandzander commented Jul 17, 2020 via email

friedrichatgc commented Jul 17, 2020

friedrichatgc commented Jul 17, 2020

GreenGary commented Jul 22, 2020

friedrichatgc commented Jul 22, 2020

rolandzander commented Jul 28, 2020 via email •

edited

Loading

friedrichatgc commented Jul 28, 2020

rolandzander commented Jul 30, 2020

friedrichatgc commented Jul 30, 2020

rolandzander commented Jul 30, 2020

friedrichatgc commented Jul 30, 2020

Support Windows using MSVC #5

Support Windows using MSVC #5

Comments

GreenGary commented Mar 30, 2020 • edited Loading

GreenGary commented Mar 30, 2020

friedrichatgc commented Mar 30, 2020

GreenGary commented Apr 13, 2020 • edited Loading

GreenGary commented Apr 18, 2020

GreenGary commented Apr 20, 2020

friedrichatgc commented Apr 20, 2020

GreenGary commented Apr 30, 2020

friedrichatgc commented Apr 30, 2020

GreenGary commented May 11, 2020

friedrichatgc commented May 12, 2020

friedrichatgc commented May 13, 2020

friedrichatgc commented Jul 16, 2020

rolandzander commented Jul 17, 2020 via email

friedrichatgc commented Jul 17, 2020

friedrichatgc commented Jul 17, 2020

GreenGary commented Jul 22, 2020

friedrichatgc commented Jul 22, 2020

rolandzander commented Jul 28, 2020 via email • edited Loading

friedrichatgc commented Jul 28, 2020

rolandzander commented Jul 30, 2020

friedrichatgc commented Jul 30, 2020

rolandzander commented Jul 30, 2020

friedrichatgc commented Jul 30, 2020

GreenGary commented Mar 30, 2020 •

edited

Loading

GreenGary commented Apr 13, 2020 •

edited

Loading

rolandzander commented Jul 28, 2020 via email •

edited

Loading