search

advanpix / mpreal

GNU General Public License v3.0
84 stars 19 forks source link

Updates to mpfr break mpreal.h #7

Closed ghborrmann closed 2 years ago

ghborrmann commented 3 years ago

Recent patches to Gnu mpfr, as implemented in Fedora's mpfr-devel version 4.1.0-6, cause compilation errors from mpreal.h. The errors come from mpfr's recently-introduced SRCPTR macro, and occur because mpfr.h and mpreal.h both use the identifier srcptr, with different definitions: it is a pointer in mpfr.h and a function in mpreal.h. The fix is fairly trivial; it is only necessary to rename mpreal's srcptr function.

koder77 commented 3 years ago

Hello, did you make a patch for mpreal.h? I did notice that the example of mpreal.h can not be compiled anymore. I use mpreal.h for my mpfrc++ math module in my L1VM VM project.

ghborrmann commented 3 years ago

I merely did a global replace of srcptr() with xsrcptr().  (178 occurrences)

On 6/9/2021 3:47 PM, Stefan Pietzonke wrote:

Hello, did you make a patch for mpreal.h? I did notice that the example of mpreal.h can not be compiled anymore. I use mpreal.h for my mpfrc++ math module in my L1VM VM project.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/advanpix/mpreal/issues/7#issuecomment-858045308, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACH7DMZB44IKMMXNPUSDISDTR7AN7ANCNFSM45ZZCLHA.

srbhp commented 3 years ago

May please share your mpreal.h file?

koder77 commented 3 years ago

Hello,

my new "mpreal.h" file is here on my GitHub repo: GitHub koder77 L1VM

cantonios commented 3 years ago

+1

MadCatX commented 3 years ago

+1

While a fix for this issue is necessary, note that renaming this MPReal function may lead to breakages elsewhere. For instance, Eigen's https://gitlab.com/libeigen/eigen/-/blob/master/unsupported/Eigen/MPRealSupport file needs to be updated to use mpfr_xsrcptr.

advanpix commented 2 years ago

Thanks all for the comments and ideas. Looks like the easiest way would be to define MPFR_USE_NO_MACRO just before including mpfr.h in mpreal.h, e.g.:

#define MPFR_USE_NO_MACRO
#include <mpfr.h>

This disables all the notorious macro in mpfr.h causing the issue, and also preserves the compatibility with Eigen. The only downside is that some utility functions, like isnan, isinf, isfinite, isnum, iszero and isint will become slower to some degree.

What do you think about this simple fix? Or do you prefer the re-naming?

MadCatX commented 2 years ago

Since Eigen is still actively developed, pushing a patch to Eigen should not be a problem. As such, I'd vote for renaming.

advanpix commented 2 years ago

As it turns out, there are a lot of software which depend on mpreal.h and its mpfr_srcptr member. Even some of the prominent commercial software packages depend on this, and renaming would be a huge PITA for all the software. I have received quite a few emails about this in the last few days.

So that, at the moment, disabling macro looks like the easiest solution. Speed regression of is** functions can be avoided by static linking to MPFR (so that linker will inline the functions anyway).

advanpix commented 2 years ago

Fixed by commit 269a03782fa0b87c20470976b88677266c5423ad