From 0741963ca520ed9ca583625f5cbc6c2bfc6c93a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Villemot?= <sebastien@dynare.org>
Date: Sun, 9 May 2021 18:00:42 +0200
Subject: [PATCH] Build system: streamline handling of compilation flags for
 MEX
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

— allow the user to override compilation flags for MATLAB MEX (it was already
  working for the preprocessor, the MEX for Octave and Dynare++)
— increase the symmetry of MEX build infrastructure between MATLAB and Octave
— when linking MEX for Octave, do not add the output of “mkoctfile -p FLIBS”.
  It is unneeded, and it can create a conflict between the system compiler and
  a user-supplied compiler

By the way:
— restore optimization on macOS for C/C++ MEX (it had been removed in
  5df2392a09d831f16e9e17c6a371652c7401f0e2)
— remove -fno-omit-frame-pointer on MATLAB/Linux, since it would be cancelled
  by subsequent -O2 and should not be needed anyways
— remove FFLAGS under Octave, unused
---
 m4/ax_mexopts.m4              | 22 +++++++++++-----------
 m4/ax_slicot.m4               |  8 +++++---
 mex/build/matlab/configure.ac | 10 ++++++----
 mex/build/matlab/mex.am       |  3 +++
 mex/build/octave/configure.ac |  8 ++++----
 mex/build/octave/mex.am       |  8 +++-----
 6 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/m4/ax_mexopts.m4 b/m4/ax_mexopts.m4
index 9b51833c65..db2276a650 100644
--- a/m4/ax_mexopts.m4
+++ b/m4/ax_mexopts.m4
@@ -30,20 +30,20 @@ MATLAB_CPPFLAGS="-I$MATLAB/extern/include"
 
 case ${MATLAB_ARCH} in
   glnxa64)
-    MATLAB_DEFS="$MATLAB_DEFS -D_GNU_SOURCE -DNDEBUG"
-    MATLAB_CFLAGS="-fexceptions -fno-omit-frame-pointer -fPIC -pthread -g -O2"
-    MATLAB_CXXFLAGS="-fno-omit-frame-pointer -fPIC -pthread -g -O2"
-    MATLAB_FCFLAGS="-fPIC -g -O2 -fexceptions"
+    MATLAB_DEFS="-D_GNU_SOURCE -DNDEBUG"
+    MATLAB_CFLAGS="-fexceptions -fPIC -pthread"
+    MATLAB_CXXFLAGS="-fPIC -pthread"
+    MATLAB_FCFLAGS="-fPIC -fexceptions"
     MATLAB_LDFLAGS_NOMAP="-shared -Wl,--no-undefined -Wl,-rpath-link,$MATLAB/bin/glnxa64 -L$MATLAB/bin/glnxa64"
     MATLAB_LDFLAGS="$MATLAB_LDFLAGS_NOMAP -Wl,--version-script,$MATLAB/extern/lib/glnxa64/mexFunction.map"
     MATLAB_LIBS="-lmx -lmex -lmat -lm -lstdc++ -lmwlapack -lmwblas"
     ax_mexopts_ok="yes"
     ;;
   win64)
-    MATLAB_CFLAGS="-fexceptions -g -O2"
-    MATLAB_CXXFLAGS="-g -O2"
-    MATLAB_FCFLAGS="-g -O2 -fexceptions -fno-underscoring"
-    MATLAB_DEFS="$MATLAB_DEFS -DNDEBUG"
+    MATLAB_CFLAGS="-fexceptions"
+    MATLAB_CXXFLAGS=""
+    MATLAB_FCFLAGS="-fexceptions -fno-underscoring"
+    MATLAB_DEFS="-DNDEBUG"
     # The hack for libquadmath is needed because -static-libgfortran
     # unfortunately does not imply the static linking of the former.
     # The last part about winpthread is a hack to avoid dynamically linking
@@ -57,10 +57,10 @@ case ${MATLAB_ARCH} in
     ax_mexopts_ok="yes"
     ;;
   maci64)
-    MATLAB_DEFS="$MATLAB_DEFS -DNDEBUG"
+    MATLAB_DEFS="-DNDEBUG"
     MATLAB_CFLAGS="-fno-common -fexceptions"
-    MATLAB_CXXFLAGS="$MATLAB_CFLAGS"
-    MATLAB_FCFLAGS="-g -O2 -fexceptions -fbackslash"
+    MATLAB_CXXFLAGS="-fno-common -fexceptions"
+    MATLAB_FCFLAGS="-fexceptions -fbackslash"
     MATLAB_LDFLAGS_NOMAP="-Wl,-twolevel_namespace -undefined error -bundle"
     MATLAB_LDFLAGS="$MATLAB_LDFLAGS_NOMAP -Wl,-exported_symbols_list,\$(abs_top_srcdir)/mexFunction-MacOSX.map"
     # This -L flag is put here, hence later on the linker command line, so as
diff --git a/m4/ax_slicot.m4 b/m4/ax_slicot.m4
index 1962555314..c3bc018e88 100644
--- a/m4/ax_slicot.m4
+++ b/m4/ax_slicot.m4
@@ -37,9 +37,11 @@ AC_DEFUN([AX_SLICOT],
   else
     LDFLAGS_SLICOT=""
   fi
-  ac_save_LDFLAGS="$LDFLAGS"
-  LDFLAGS_SAVED="$LDFLAGS"
+  ac_save_LDFLAGS=$LDFLAGS
 
+  # At this point we should add MATLAB_FCFLAGS to FCFLAGS for Windows (which has -fno-underscoring),
+  # but that does not work. The actual underscore test seems to happen at the very beginning of the
+  # macro. Hence the modification of FCFLAGS was moved higher (in mex/build/matlab/configure.ac).
   AC_FC_FUNC(sb02od)
 
   if test "$1" = matlab; then
@@ -56,7 +58,7 @@ AC_DEFUN([AX_SLICOT],
              [$($MKOCTFILE -p BLAS_LIBS) $($MKOCTFILE -p LAPACK_LIBS)])
   fi
 
-  LDFLAGS="$ac_save_LDFLAGS"
+  LDFLAGS=$ac_save_LDFLAGS
   AC_SUBST(LDFLAGS_SLICOT)
   AC_SUBST(LIBADD_SLICOT)
 ])
diff --git a/mex/build/matlab/configure.ac b/mex/build/matlab/configure.ac
index c9460676a4..63ddf61a24 100644
--- a/mex/build/matlab/configure.ac
+++ b/mex/build/matlab/configure.ac
@@ -45,10 +45,6 @@ case ${host_os} in
     ;;
 esac
 
-CFLAGS="$MATLAB_CFLAGS -Wall -Wno-parentheses"
-FCFLAGS="$MATLAB_FCFLAGS -Wall"
-CXXFLAGS="$MATLAB_CXXFLAGS -Wall -Wno-parentheses -Wold-style-cast"
-
 AC_PROG_FC
 AC_PROG_CC
 AC_PROG_CC_C99
@@ -89,7 +85,13 @@ fi
 
 # Check for libslicot, needed by kalman_steady_state
 if test "$enable_mex_kalman_steady_state" = yes; then
+  # FCFLAGS must be temporarily modified, because otherwise -fno-underscoring is not
+  # taken into account by AC_FC_FUNC in the AX_SLICOT macro.
+  # For some obscure reason, it is necessary to do it at this level and not within the macro.
+  ac_save_FCFLAGS=$FCFLAGS
+  FCFLAGS="$FCFLAGS $MATLAB_FCFLAGS"
   AX_SLICOT([matlab])
+  FCFLAGS=$ac_save_FCFLAGS
   test "$has_slicot" != yes && AC_MSG_ERROR([slicot cannot be found. If you want to skip the compilation of the kalman_steady_state MEX, pass the --disable-mex-kalman-steady-state flag.])
 fi
 
diff --git a/mex/build/matlab/mex.am b/mex/build/matlab/mex.am
index 5ba01b7230..0bd51ad0bf 100644
--- a/mex/build/matlab/mex.am
+++ b/mex/build/matlab/mex.am
@@ -7,6 +7,9 @@ DEFS += $(MATLAB_DEFS)
 DEFS += -DMATLAB_MEX_FILE
 DEFS += -DMEXEXT=\"$(MEXEXT)\"
 
+AM_CFLAGS = $(MATLAB_CFLAGS) -Wall -Wno-parentheses
+AM_FCFLAGS = $(MATLAB_FCFLAGS) -Wall
+AM_CXXFLAGS = $(MATLAB_CXXFLAGS) -Wall -Wno-parentheses -Wold-style-cast
 AM_LDFLAGS = $(MATLAB_LDFLAGS)
 LIBS += $(MATLAB_LIBS)
 
diff --git a/mex/build/octave/configure.ac b/mex/build/octave/configure.ac
index b9ffc14ce2..1ce99dbb64 100644
--- a/mex/build/octave/configure.ac
+++ b/mex/build/octave/configure.ac
@@ -26,14 +26,14 @@ AX_OCTAVE
 
 test "$ax_enable_octave" != yes && AC_MSG_ERROR([Octave cannot be found])
 
+# Let mkoctfile set the default compilers and flags (except for FC and FCFLAGS which are not supported)
+# NB: mkoctfile honors overrides via environment variables
 CC=$($MKOCTFILE -p CC)
 CXX=$($MKOCTFILE -p CXX)
 AR=$($MKOCTFILE -p AR)
 RANLIB=$($MKOCTFILE -p RANLIB)
-CFLAGS="$($MKOCTFILE -p CFLAGS) -Wall -Wno-parentheses"
-FCFLAGS="$($MKOCTFILE -p FFLAGS) -Wall -std=gnu" # Override -std=legacy that is in FFLAGS in Octave 5
-FFLAGS="$($MKOCTFILE -p FFLAGS) -Wall"
-CXXFLAGS="$($MKOCTFILE -p CXXFLAGS) -Wall -Wno-parentheses -Wold-style-cast"
+CFLAGS=$($MKOCTFILE -p CFLAGS)
+CXXFLAGS=$($MKOCTFILE -p CXXFLAGS)
 LDFLAGS="$($MKOCTFILE -p LFLAGS) $($MKOCTFILE -p LDFLAGS)"
 
 AC_CANONICAL_HOST
diff --git a/mex/build/octave/mex.am b/mex/build/octave/mex.am
index 77eccb3e7d..4766d5d25f 100644
--- a/mex/build/octave/mex.am
+++ b/mex/build/octave/mex.am
@@ -5,10 +5,9 @@ AM_CPPFLAGS += -I$(top_srcdir)/../../sources
 DEFS += -DOCTAVE_MEX_FILE
 DEFS += -DMEXEXT=\".mex\"
 
-AM_CFLAGS = $(shell $(MKOCTFILE) -p CPICFLAG)
-AM_FFLAGS = $(shell $(MKOCTFILE) -p FPICFLAG)
-AM_FCFLAGS = $(shell $(MKOCTFILE) -p FPICFLAG)
-AM_CXXFLAGS = $(shell $(MKOCTFILE) -p CXXPICFLAG)
+AM_CFLAGS = $(shell $(MKOCTFILE) -p CPICFLAG) -Wall -Wno-parentheses
+AM_FCFLAGS = $(shell $(MKOCTFILE) -p FPICFLAG) -Wall
+AM_CXXFLAGS = $(shell $(MKOCTFILE) -p CXXPICFLAG) -Wall -Wno-parentheses -Wold-style-cast
 AM_LDFLAGS = $(shell $(MKOCTFILE) -p DL_LDFLAGS) -L"$(shell $(MKOCTFILE) -p OCTLIBDIR)"
 
 LIBS += $(shell $(MKOCTFILE) -p OCTAVE_LIBS)
@@ -16,7 +15,6 @@ LIBS += $(shell $(MKOCTFILE) -p BLAS_LIBS)
 LIBS += $(shell $(MKOCTFILE) -p LAPACK_LIBS)
 LIBS += $(shell $(MKOCTFILE) -p FFTW_LIBS)
 LIBS += $(shell $(MKOCTFILE) -p LIBS)
-LIBS += $(shell $(MKOCTFILE) -p FLIBS)
 
 mexdir = $(libdir)/dynare/mex/octave
 
-- 
GitLab