Skip to content
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

makefile and header fixes for Unix #6

Merged
merged 1 commit into from
Feb 27, 2018

Conversation

dimpase
Copy link

@dimpase dimpase commented Feb 27, 2018

No description provided.

@dimpase
Copy link
Author

dimpase commented Feb 27, 2018

@wbhart - here it is

@wbhart
Copy link

wbhart commented Feb 27, 2018

This PR is against Brian's repo (which is fine). I can't merge it there. Of course I'm happy to merge it from Brian (he can make a PR against the main MPIR repo when he is happy, and I'll merge it).

@wbhart
Copy link

wbhart commented Feb 27, 2018

The other option of course is to change Brian's repo to the official one. The only job I'm doing at present is merging PRs (and trying to keep broken code out).

@dimpase
Copy link
Author

dimpase commented Feb 27, 2018

On OSX mpn/t-gcdext test fails. (Linux tests all pass)

@dimpase
Copy link
Author

dimpase commented Feb 27, 2018

The other option of course is to change Brian's repo to the official one

As I already mentioned, it perhaps is better to make an MPIR Github organisation, and host the main repo there. This would make it more professional-looking, and properly reflect that it's not an individual project.

@wbhart
Copy link

wbhart commented Feb 27, 2018

Yes, you could do that. I think the previous proposal was that it be made a part of Nemocas, which we don't want. But there's no reason not to have an MPIR GitHub organisation.

I had a look at the OSX logs. There's some warnings there which all look serious. But I don't see any obvious cause for the test failure. It might be worth capturing the test program log (it's piped to a file by MPIR), in case it is a real (heisen) bug.

By the way, Brian added other files (that aren't specific to Windows) on the Windows side. Were they added also on the *nix side? Autotools doesn't pick them up automatically, so they won't even make it into the tarball when you do make dist.

@BrianGladman BrianGladman merged commit 732e10c into BrianGladman:master Feb 27, 2018
@BrianGladman
Copy link
Owner

Great work Dima!

@BrianGladman
Copy link
Owner

I think Dima's idea of MPIR as a GITHUB project in its own right makes sense.

@wbhart
Copy link

wbhart commented Feb 27, 2018

I agree. Anyone can set that up. I can change the link on the website if anyone does it.

I would give someone else the password for the website, but I can't. Fredrik may be able to give access to someone else though.

@BrianGladman
Copy link
Owner

BrianGladman commented Feb 27, 2018

The functions I added are mpf_cmp_z, mpn_zero_p and mpn_perfect_power_p.

I added two other functions, but on Windows only: mpz_get_2exp_d and mpf_get_2exp_d. These do the same work as mpz_get_d_2exp and mpf_get_d_2exp but avoid the problems caused by having pointers to 32-bit types being used to store 64-bit values on Windows x64. I don't think these have any imapct on Linux/GCC as they won't be present.

I have revised the documentation of the Windows build in the documentation and I have also added the new function descriptions except for mpn_pefect_power_p (because this was missing in the GMP documentation).

@BrianGladman
Copy link
Owner

Sadly I am still seeing failures on Travis :-(

@dimpase
Copy link
Author

dimpase commented Feb 27, 2018 via email

@BrianGladman
Copy link
Owner

BrianGladman commented Feb 27, 2018

Thanks Dima - its approaching midnight here - I will continue with anything needed from me in the morning. I guess we still have to think about the GIT sub-module issue. But the use of a single msvc sub-directory has cleaned things up a lot anyway so its less of an issue now (I hope!).

@dimpase
Copy link
Author

dimpase commented Feb 27, 2018 via email

@BrianGladman
Copy link
Owner

OK, I'll do that in the morning when I am more awake than I am now!

@BrianGladman
Copy link
Owner

Apart from the contents of the msvc sub-directory, I have only added three files: mpf/cmp_z.c, mpn/generic/perfpow.c and mpn/generic/zero_p. I imported the latter from GMP but it is not documented in their manual. However, they use it as a basis for their mpz_perfect_power_p implementation whereas we have our own version of this.

So I am now wondering whether we should (a) not bother with mpn_perfect_power_p, (b) keep both of the mpn and mpz versions, or (c) implement our mpz function by calling the mpn version. I haven't been able to test their relative speed yet but I wonder if anyone hs views on what we should do here?

An issue that I have not done anything about is to add the new functions to the speed program - adding new functions to it is a bit of a mystery to me as I haven't played with it.

@dimpase
Copy link
Author

dimpase commented Feb 28, 2018

One failure we see on OSX is surely due to mpn/generic/zero_p not being properly entered into autoconf/automake configs. But I don't understand how it should be done, mpn/ seems to have a less straightforward than mpf/ way to deal with this.

@wbhart - can you comment on this?

less tests/mpn/t-gcdext.log 
dyld: lazy symbol binding failed: Symbol not found: ___gmpn_zero_p
  Referenced from: /Users/dimpase/Sage/mpir/.libs/libmpir.23.dylib
  Expected in: flat namespace

dyld: Symbol not found: ___gmpn_zero_p
  Referenced from: /Users/dimpase/Sage/mpir/.libs/libmpir.23.dylib
  Expected in: flat namespace

FAIL t-gcdext (exit status: 134)

We don't see this on Linux as a non-generic implementation is chosen.

@wbhart
Copy link

wbhart commented Feb 28, 2018

Yes, GMP/MPIR has a custom process for building the mpn part of the library. there are devel docs in the repository which explain how to do all this. The process is necessarily quite complex due to the way assembly functions for multiple arches are dealt with and the complexities of fat binaries and conditional inclusion of certain assembly optimisations.

@dimpase
Copy link
Author

dimpase commented Feb 28, 2018

can you point out to a file in mpn/generic/ that is used and dealt with properly, which I can use as an example?

@wbhart
Copy link

wbhart commented Feb 28, 2018

The files are handled differently depending on what kind of function it is. But if the file is only present in generic, the documentation covers this straightforwardly.

As an example, inv_div_q.c is a generic only function. You should be able to grep configure.ac and the Makefiles to see how this is handled.

Of course files don't necessarily just drop straight in to MPIR from GMP. But Brian has surely already made any changes that are necessary, so that it works on the Windows side.

@BrianGladman
Copy link
Owner

Yes, I have made all the changes that I believe are needed other than those within the Linux/GCC build system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants