search

flintlib / flint

FLINT (Fast Library for Number Theory)
http://www.flintlib.org
GNU Lesser General Public License v3.0
424 stars 241 forks source link

HAVE_GC might remain undefined in 2.6.3 (if built with cmake) #837

Closed dimpase closed 3 years ago

dimpase commented 4 years ago

In Flint 2.5* (and even later, maybe, but not in 2.6.3) the macro HAVE_GC was always defined, and set to 0 or 1. Now if Flint is built with cmake, then instead of 

$ grep -R HAVE_GC
flint-config.h:#define HAVE_GC 0

one just sees no HAVE_GC in flint-config.h. This is a kind of a bug.

It is in fact just not possible to tell cmake to use GC with Flint, as of Flint 2.6.3 or earlier.

dimpase commented 4 years ago

@isuruf

dimpase commented 4 years ago

it broke recognition of flint in Sage on Gentoo, where flint is built with cmake, although we can certainly fix it.

isuruf commented 4 years ago

You need something like https://github.com/wbhart/flint2/commit/720fc48ebe52b6a57db6bdfc2cec58e0b2bf07b6

isuruf commented 4 years ago

Also see https://github.com/wbhart/flint2/pull/831

dimpase commented 4 years ago

How does one even requests the equivalent of --with-gc=... with cmake ? E.g. the current Gentoo package doesn't even have a flag for it.

kiwifb commented 3 years ago

Right, I am sorry I skim a lot of stuff these days. So with cmake, let's look at https://github.com/wbhart/flint2/blob/trunk/CMakeLists.txt#L174 and later. This is where the MEMORY_MANAGER is set by default to reentrant when it can be single, reentrant or gc. So to change it, you would have set something like -DMEMORY_MANAGER="gc" I think. If we look further in CMakeLists.txt we can see that two files sources are different depending on MEMORY_MANAGER. Look at https://github.com/wbhart/flint2/blob/trunk/CMakeLists.txt#L189 and https://github.com/wbhart/flint2/blob/trunk/CMakeLists.txt#L195. I elected to just use the default with cmake. It felt decent enough and from what I can see this is actually a tri-state config and those are a bit harder to handle.

kiwifb commented 3 years ago

And it seems that reentrant is identical to gc at first glance. And there is nothing to find gc in cmake. Possibly not implemented yet?

dimpase commented 3 years ago

@kiwifb - any reason Gentoo wants to use cmake ?

Indeed, it looks like it's not implemented. There is no check for libgc in CMake/, whereas there are such checks for NTL, mpfr, gmp... And there is nothing in CMake files that flips HAVE_CG to 0 or 1. I'd say it's a bug.

kiwifb commented 3 years ago

Honestly? cmake or custom build system that you don't know when things will go wrong and is a potential huge portability issue? Should I even hesitate?

dimpase commented 3 years ago

cmake is a hugely oversold custom build system IMHO. :-) Just look how much handwritten code one has here, and how verbose it is...

kiwifb commented 3 years ago

Not raising to such a bet before bedtime :) in any case it probably wouldn't hurt to throw an appropriate define in there, for compatibility's sake (spell checker suggestions of the day: "compatibility shake") at least.

mahrud commented 3 years ago

Is there a particular reason to use gc rather than reentrant?

This is probably fairly easy to fix, along with other differences that might be more significant, like HAVE_PTHREAD, which isn't set by CMake.

cmake is a hugely oversold custom build system IMHO. :-) Just look how much handwritten code one has here, and how verbose it is...

In comparison to autotools? CMakeLists.txt is 365 lines, which is a third of configure+Makefile.in+Makefile.subdirs at 1178 lines, all of whom are handwritten.

dimpase commented 3 years ago

In comparison to autotools? CMakeLists.txt is 365 lines

sure, in comparison to autotools (autoconf+automake+...) cmake is quite verbose. No autotools here, so it's unfair to count lines - besides, you forgot 238 lines in CMake/*.cmake :-)

kiwifb commented 3 years ago

I am quite ready to throw gc under the proverbial bus in exchange to be able to use cmake to build. reentrant seems a fine choice to me. Especially, if don't use any gc specific features anyway.

wbhart commented 3 years ago

Unless Sage uses it, I see no reason to support --with-gc with CMake. The option only exists for the sake of Macaulay 2 I think, and I am not sure how much longer they will be using it, if they still are. They obviously don't use CMake to build Flint if they are still using the option.

dimpase commented 3 years ago

I checked today, Macaulay2 stopped using --with-gc 2 years ago. And, by the way, they are switching to cmake :-)

wbhart commented 3 years ago

Excellent. I never liked that hack anyway!

mahrud commented 3 years ago

I noticed a couple of other differences:

  1. CMake checks for pthreads, but never sets HAVE_PTHREAD to 1 when it is present
  2. CMake doesn't set POPCNT_INTRINSICS, HAVE_BLAS (which is actually never used), and HAVE_FENV

The pthreads one seems significant, doesn't it?

wbhart commented 3 years ago

All of those are now relevant. We will even shortly be using BLAS (Dan already has a prototype).

wbhart commented 3 years ago

If one of the CMake maintainers addresses this with a PR this week, I'll put it in 2.7.0. Otherwise it's going to be pushed out to 2.8.0.

wbhart commented 3 years ago

I am referring to mahrud's final comment above, especially his point 1. I don't think the original ticket is relevant at all (it relates to a feature only used by Macaulay 2 two years ago, but not any more).