Skip to content

Better interoperability between MSVC and mingw-w64, 2 #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 220 commits into from

Conversation

nijtmans
Copy link
Collaborator

@nijtmans nijtmans commented Apr 9, 2019

Here is another attempt, which succeeded to produce a single "libtommath.dll" (actually 2: win32 and win64) which is usable for both MSVC and mingw-w64. The dll can be built normally, only an addtional compiler option prevents that it depends on a mingw-w64 specific dll.

I built Tcl with MSVC (both win32 and win64) and with mingw-w64 (also win32 and win64), but replacing the built-in libtommath with "libtommath.dll". It worked. The MSVC import library "tommath.lib" must be generated using a def-file (tommath.def), which is also committed. Ideally, this .def-file should be automatically generated (as is done with the makefiles), but I'm not confident enough in perl to implement that.

Advantage of this approach: MSVC-built application can benefit from DIGIT_BITS=60, even though the compiler cannot build the dll. But it can import the mingw-w64-generated dll, and fully interoperate with it.

@sjaeckel
Copy link
Member

how's this def file created?

@nijtmans nijtmans requested a review from sjaeckel April 10, 2019 07:27
@nijtmans
Copy link
Collaborator Author

nijtmans commented Apr 10, 2019

how's this def file created?

Manually, just by building first and then using nm to make a list of the exported symbols. It just contains a list of exported symbols in alphabetical order (not really necessary, actually). Should be generated by a script, but I'm not confident in perl doing that.

@sjaeckel
Copy link
Member

Manually, just by building first and then using nm to make a list of the exported symbols. It just contains a list of exported symbols in alphabetical order (not really necessary, actually). Should be generated by a script, but I'm not confident in perl doing that.

no need to do it in perl, I'd use the same tools as @minad did in rename.sh in #172 as I'm also not confident in perl :)

I think this script needs to do more in the future, but what you described is enough for now.

In the future (2.0.0) it should build the library, then check vs. what is in tommath.h (and don't care which functions come out of tommath_private.h) and only add those of tommath.h to the list.

@sjaeckel
Copy link
Member

can you please rebase on top of develop and squash all those commits?

@sjaeckel sjaeckel requested a review from karel-m April 11, 2019 12:28
@nijtmans nijtmans force-pushed the interop-mingw-w64-2 branch from aefcf64 to fbc0829 Compare April 11, 2019 14:01
@nijtmans
Copy link
Collaborator Author

can you please rebase on top of develop and squash all those commits?

Done @sjaeckel

@sjaeckel
Copy link
Member

Thx!

@karel-m can you please have a look at these changes, fine by you as well?

@minad
Copy link
Member

minad commented Apr 12, 2019

MSVC-built application can benefit from DIGIT_BITS=60, even though the compiler cannot build the dll. But it can import the mingw-w64-generated dll, and fully interoperate with it.

@nijtmans I had expected that something like this might work. However you mentioned that tcl accesses the mp_int structs. How will that work?

@minad
Copy link
Member

minad commented Apr 12, 2019

Ah, and why do you need a def file? Just asking, I don't know what they are for. Some kind of additional infos for the linker?

@nijtmans
Copy link
Collaborator Author

MSVC-built application can benefit from DIGIT_BITS=60, even though the compiler cannot build the dll. But it can import the mingw-w64-generated dll, and fully interoperate with it.

@nijtmans I had expected that something like this might work. However you mentioned that tcl accesses the mp_int structs. How will that work?

Well, it works: See here. As long as Tcl is compiled with the same MP-mode as the libtommath library, it works. (There were a few fixes needed in the Tcl source-code, but those are done now)

@nijtmans
Copy link
Collaborator Author

Ah, and why do you need a def file? Just asking, I don't know what they are for. Some kind of additional infos for the linker?

A def-file is needed for MSVC-builds, the import-libraries can be generated from it.

@nijtmans nijtmans requested a review from minad April 12, 2019 09:45
minad and others added 24 commits May 26, 2019 11:47
Msvc fixes (appveyor works!)
These functions were introduced to give some timing guarantees.
However the guarantees are too weak to be useful.
The functions seem to be unused essentially by downstream users.
deprecate mp_n_root_ex and mp_expt_d_ex
…what symbols are there. Re-generation script should deliver the same file.
Re-generate tommath.def, by actually building with mingw-w64 and see what symbols are there. Re-generation script should deliver the same file.
@nijtmans
Copy link
Collaborator Author

Well, I tried to rebase, but somehow that failed horribily

@sjaeckel
Copy link
Member

Well, I tried to rebase, but somehow that failed horribily

hmm I tried the rebase of the old HEAD @ fbc0829 and there was a minor merge conflict, but otherwise it went smoothly

now that we have bn_deprecated.c et.al. your old approach of searching for the files etc also won't work anymore...

@sjaeckel
Copy link
Member

hmm I tried the rebase of the old HEAD @ fbc0829 and there was a minor merge conflict, but otherwise it went smoothly

I've pushed the rebased branch to rebased/interop-mingw-w64-2

@nijtmans
Copy link
Collaborator Author

Closing in favour of #300

@nijtmans nijtmans closed this May 28, 2019
@sjaeckel sjaeckel deleted the interop-mingw-w64-2 branch May 28, 2019 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants