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

[QUESTION] Are bounds checking and floating-point exceptions included in CMake build options? #303

Closed
msulprizio opened this issue May 11, 2020 · 5 comments
Assignees
Labels
category: Question Further information is requested
Milestone

Comments

@msulprizio
Copy link
Contributor

I am trying to chase down what I think is an array-out-of-bounds error, so I've turned on the debug option in CMake. However, it isn't returning an information which led me to wonder if the debug option actually turns on checking bounds. I don't see any reference to bounds checking or floating point exceptions in the CMake files. We use both of those options regularly in debugging GEOS-Chem (that is the BOUNDS=y FPE=y switches in GNU Make) so we should also include them in CMake. In Makefile_Header.mk:

  # Turn on checking for floating-point exceptions                                        
  # These are approximately equivalent to -fpe0 -ftrapuv in IFORT                         
  # NOTE: GNU Fortran 4.4.7 does not allow for -finit-real-snan, so                       
  # we will only add this flag for versions newer than 4.4.7                              
  REGEXP             :=(^[Yy]|^[Yy][Ee][Ss])
  ifeq ($(shell [[ "$(FPE)" =~ $(REGEXP) ]] && echo true),true)
    FFLAGS           += -ffpe-trap=invalid,zero,overflow
    ifeq ($(NEWER_THAN_447),1)
      FFLAGS           += -finit-real=snan
    endif
  endif
  ifeq ($(shell [[ "$(FPEX)" =~ $(REGEXP) ]] && echo true),true)
    FFLAGS           += -ffpe-trap=invalid,zero,overflow
    ifeq ($(NEWER_THAN_447),1)
      FFLAGS           += -finit-real=snan
    endif
  endif
  # Add option for "array out of bounds" checking                                         
  REGEXP             := (^[Yy]|^[Yy][Ee][Ss])
  ifeq ($(shell [[ "$(BOUNDS)" =~ $(REGEXP) ]] && echo true),true)
    FFLAGS           += -fbounds-check
  endif

@LiamBindle can you confirm that (1) these options aren't already included in GEOS-Chem Classic's CMake options so far? and (2) advise on the best way to add this capability? If I understand correctly, I think we need to add to GC-DefaultCompilerOptions.cmake. Does it makes sense to include these in the debug option or as separate options?

@msulprizio msulprizio added the category: Question Further information is requested label May 11, 2020
@LiamBindle
Copy link
Contributor

LiamBindle commented May 11, 2020

@msulprizio Yeah, you're correct, and GC-DefaultCompilerOptions.cmake is the right place to add these. I think it makes sense to add these--initially I omitted most of the debug flags with the idea that we'd add them on a need-to-use basis (to avoid carrying forward any old unsed flags). Feel free to update these to whatever you think is appropriate.

ifort's debug options for build type Debug are set here:

set(CMAKE_Fortran_FLAGS_DEBUG "-g -O0 -check arg_temp_created -debug all -DDEBUG")

and gfortran's options for build type Debug are set here:
set(CMAKE_Fortran_FLAGS_DEBUG "-g -gdwarf-2 -gstrict-dwarf -O0 -Wall -Wextra -Wconversion -Warray-temporaries -fcheck-array-temporaries")

@LiamBindle
Copy link
Contributor

LiamBindle commented May 11, 2020

Oh, one more thing that I think might be relevent. Just so you know, you can set extra compiler options for your build directory by setting CMAKE_Fortran_FLAGS (see here). I think the flags that you're talking about should be added to the DEBUG flags, but I just wanted to mention this feature to increase awareness of it. This feature is useful for building GEOS-Chem with uncommon compiler options, like compiler options for using TAU or architecture-specific optimizations.

@msulprizio msulprizio added this to the 12.8.1 milestone May 11, 2020
@msulprizio
Copy link
Contributor Author

Thanks, @LiamBindle. This seems straightforward, so I will add bounds checking and FPE to the debug option in 12.8.1.

@msulprizio msulprizio self-assigned this May 11, 2020
@lizziel
Copy link
Contributor

lizziel commented May 11, 2020

Also heads up that file geos-chem/CMakeScripts/GC-DefaultCompilerOptions.cmake is deleted starting 13.0.0. Once this update goes into 12.8.1 we should also port it to dev/13.0.0.

@msulprizio
Copy link
Contributor Author

This is now resolved in bc0c69f and will be included in 12.8.1.

The same fix has also been pushed to dev/13.0.0 in commit 2d0a7cb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants