Opened 13 years ago
Closed 12 years ago
#8112 closed defect (fixed)
flint fails to build in Open Solaris x64 as 64 bit since FLINT_TUNE is not set
Reported by: | jsp | Owned by: | drkirkby |
---|---|---|---|
Priority: | major | Milestone: | sage-4.3.4 |
Component: | porting | Keywords: | |
Cc: | Merged in: | sage-4.3.4.alpha1 | |
Authors: | Jaap Spies | Reviewers: | David Kirkby |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
flint-1.5.0.p3 fails to build if SAGE64=yes and no CFLAGS and CFLAG64 are set globally due to a 32/64 bit issue.
A patch is on it's way.
http://boxen.math.washington.edu/home/jsp/ports/flint-1.5.0.p4.spkg
Jaap
Attachments (1)
Change History (9)
comment:1 Changed 13 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:2 follow-up: ↓ 3 Changed 13 years ago by
- Reviewers set to David Kirkby
- Status changed from needs_review to needs_work
It's not clear to me what is intended here.
- There is no such thing as CXXFLAG. Did you mean CXXFLAGS?
- The point of CFLAG64 is that it would be set to whatever flag is needed to build 64 bit executables with a C compiler, which -m64 in most, but not all cases.
- The point of CXXFLAG64 is that it would be set to whatever flag is needed to build 64 bit executables with a C++ compiler, which -m64 in most, but not all cases.
If Flint will build by setting the environment variables CFLAG64 and CXXFLAG64 to -m64, then I would not bother patching Flint at all. Just set those two environments variables. They will not mess up the build process in any way, and since they are variables, they could be set to something else. Setting CFLAGS is known to cause problems, but setting CFLAG64 or CXXFLAG64 will not cause any problems.
If you do need to patch this again, then you can remove this code:
./test_gcc_version.sh if [ $? -ne 0 ]; then echo "GCC version less than 3.4.0" echo "Flint will not be able to compile successfully" exit 1 fi
since 'prereq' will ensure that gcc is at least 4.0.1. That also means the file test_gcc_version.sh can be removed, as it is now redundant.
Thinking about this 64-bit patching process in general, it is better to simply use CFLAG64 and CXXFLAG64 rather than -m64, and simply document the user must type
$ CFLAG64=-m64 $ export CFLAG64 $ CXXFLAG64=-m64 $ export CXXFLAG64
until such time as sage-env is updated. At which point, that will be unnecessary to do. That will avoid -m64 ever having to be hard-coded again, which is a good thing, as not all compilers accept that flag.
I suspect this ticket can be closed as 'wontfix' as no fixes are needed, but I may be wrong.
Dave
comment:3 in reply to: ↑ 2 Changed 13 years ago by
Replying to drkirkby:
It's not clear to me what is intended here.
- There is no such thing as CXXFLAG. Did you mean CXXFLAGS?
I don't see anything alike in the patch file!?
- The point of CFLAG64 is that it would be set to whatever flag is needed to build 64 bit executables with a C compiler, which -m64 in most, but not all cases.
- The point of CXXFLAG64 is that it would be set to whatever flag is needed to build 64 bit executables with a C++ compiler, which -m64 in most, but not all cases.
If Flint will build by setting the environment variables CFLAG64 and CXXFLAG64 to -m64, then I would not bother patching Flint at all. Just set those two environments variables. They will not mess up the build process in any way, and since they are variables, they could be set to something else. Setting CFLAGS is known to cause problems, but setting CFLAG64 or CXXFLAG64 will not cause any problems.
Are you shure flint will build that way?
If you do need to patch this again, then you can remove this code:
./test_gcc_version.sh if [ $? -ne 0 ]; then echo "GCC version less than 3.4.0" echo "Flint will not be able to compile successfully" exit 1 fisince 'prereq' will ensure that gcc is at least 4.0.1. That also means the file test_gcc_version.sh can be removed, as it is now redundant.
Thinking about this 64-bit patching process in general, it is better to simply use CFLAG64 and CXXFLAG64 rather than -m64, and simply document the user must type
$ CFLAG64=-m64 $ export CFLAG64 $ CXXFLAG64=-m64 $ export CXXFLAG64until such time as sage-env is updated. At which point, that will be unnecessary to do. That will avoid -m64 ever having to be hard-coded again, which is a good thing, as not all compilers accept that flag.
I suspect this ticket can be closed as 'wontfix' as no fixes are needed, but I may be wrong.
We'll see.
Dave
comment:4 Changed 13 years ago by
- Status changed from needs_work to needs_review
Maybe the description was somewhat misleading or I was unclear over the meaning of this.
What I propose is:
if [ "x$SAGE64" = xyes ]; then FLINT_TUNE=" -fPIC -m64 -funroll-loops" export CFLAGS="$CFLAGS -m64" export CXXFLAGS="$CXXFLAGS -m64" export CFLAG64="$CFLAG64 -m64" export CXXFLAG64="$CXXFLAG -m64" fi
Now if CFLAGS is empty the -m64 gets in.
See the patch.
Jaap
comment:5 Changed 12 years ago by
- Status changed from needs_review to needs_work
This has been a bit messed up by me, as I made some changes which assumed we could set CFLAGS globally, but we can't.
I'd proposed using CFLAG64 to be the C compiler flag(s) needed to build a 64-bit binary, which are usually -m64, but might not be. That could be set globally, but lets assume for now it is not.
How would something like this seem:
If [ -z "$CFLAG64" ] ; then
CFLAG64=-m64
fi
(see how I did this in #8191)
http://trac.sagemath.org/sage_trac/attachment/ticket/8191/R.patch
Then simply append $CFLAG64 to FLINT_TUNE. But remove the -m64, as we don't need both.
Since CFLAGS and CXXFLAGS are not set in spkg-install, there is no point exporting them. Neither is there any point in exporting CFLAG64 or CXXFLAG64 as again they are not used by Flint.
I think all that needs to be done is to set CFLAG64 to be -m64 unless it is set to something else, then ensure that for a 64-bit build, FLINT_TUNE gets $CFLAG64 included.
Dave
comment:6 Changed 12 years ago by
- Status changed from needs_work to needs_review
Done as you suggested. We pass $CFLAG64 to FLINT_TUNE if SAGE64=yes. But export CXXFLAG64 appropriate. See makefile.
# Since this code uses the C++ compiler as a linker to produce # a library, the -m64 (or equivalent) option must be provided, as it # it is in the line above where the target is libflint.dylib64 libflint.so: $(FLINTOBJ) $(CPP) $(CXXFLAG64) -fPIC -shared -o libflint.so $(FLINTOBJ) $(LIBS)
Found gcc 4 or later g++ -m64 -fPIC -shared -o libflint.so zn_mod.o misc.o mul_ks.o pack.o mul.o mulmid.o mulmid_ks.o ks_support.o mpn_mulmid.o nuss.o pmf.o pmfvec_fft.o tuning.o mul_fft.o mul_fft_dft.o array.o invert.o mpn_extras.o mpz_extras.o memory-manager.o ZmodF.o ZmodF_mul.o ZmodF_mul-tuning.o fmpz.o fmpz_poly.o mpz_poly-tuning.o mpz_poly.o ZmodF_poly.o long_extras.o zmod_poly.o theta.o zmod_mat.o F_mpz.o tinyQS.o factor_base.o poly.o sieve.o linear_algebra.o block_lanczos.o NTL-interface.o -L/export/home/jaap/Downloads/sage-4.3.3.alpha1/local/lib/ -L/export/home/jaap/Downloads/sage-4.3.3.alpha1/local/lib/ -lgmp -lpthread -lntl -lm Deleting old FLINT Installing new library file
New spkg with the same name: http://boxen.math.washington.edu/home/jsp/ports/flint-1.5.0.p4.spkg
Jaap
Changed 12 years ago by
comment:7 Changed 12 years ago by
- Status changed from needs_review to positive_review
- Summary changed from flint fails to build in Open Solaris x64 as 64 bit if CFLAGS is not set to flint fails to build in Open Solaris x64 as 64 bit since FLINT_TUNE is not set
Your fix resolves the issues we have, and is unlikely to break anything (see below for a possible exception). I do have a few comments that are worth documenting.
- I was a bit concerned that -funroll-loops will be enabled when SAGE64 is set to yes, despite the fact that it supposed to crash on an UltraSPARC III+ processor. (Previously -funroll-loops was disabled on Solaris SPARC). However, testing showed this will not build on Solaris 10 in 64-bit mode on SPARC, irrespective of whether -funroll-loops is set or not. Therefore the inclusion of -funroll-loops is not causing any extra problems on SPARC, and might actually improve performance when the issues are resolved on 64-bit SPARC.
- There was no need to export CXXFLAG64, as Flint will not use it, but it can do no harm whatsoever.
- I'm changing the title slightly, from CFLAGS to FLINT_TUNE, as CFLAGS is not used directly in the spkg-install. It is FLINT_TUNE that gets set
drkirkby@redstart:~/fresh/sage-4.3.3/spkg/standard/flint-1.5.0.p4$ grep CFLAGS spkg-install drkirkby@redstart:~/fresh/sage-4.3.3/spkg/standard/flint-1.5.0.p4$
Positive review.
comment:8 Changed 12 years ago by
- Merged in set to sage-4.3.4.alpha1
- Resolution set to fixed
- Status changed from positive_review to closed
Here is an spkg: http://boxen.math.washington.edu/home/jsp/ports/flint-1.5.0.p4.spkg
Jaap