From 4155d918bb2b0ef7c902cad9df9a58565413387b Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Mon, 4 Jan 2021 18:31:48 -0500 Subject: [PATCH 1/4] Disable LTO if -fno-lto is in *FLAGS environment. --- setup.py | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/setup.py b/setup.py index 26d799813f9f..f4af218f100b 100644 --- a/setup.py +++ b/setup.py @@ -106,21 +106,40 @@ def add_optimization_flags(self): """ env = os.environ.copy() - if not setupext.config.getboolean('libs', 'enable_lto', fallback=True): - return env if sys.platform == 'win32': return env - + enable_lto = setupext.config.getboolean('libs', 'enable_lto', + fallback=None) + + if 'CFLAGS' in os.environ: + if '-fno-lto' in os.environ['CFLAGS']: + if enable_lto is True: + raise ValueError('Configuration enable_lto=True, but ' + 'CFLAGS contains -fno-lto') + enable_lto = False cppflags = [] if 'CPPFLAGS' in os.environ: cppflags.append(os.environ['CPPFLAGS']) + if '-fno-lto' in os.environ['CPPFLAGS']: + if enable_lto is True: + raise ValueError('Configuration enable_lto=True, but ' + 'CPPFLAGS contains -fno-lto') + enable_lto = False cxxflags = [] if 'CXXFLAGS' in os.environ: cxxflags.append(os.environ['CXXFLAGS']) + if '-fno-lto' in os.environ['CXXFLAGS']: + if enable_lto is True: + raise ValueError('Configuration enable_lto=True, but ' + 'CXXFLAGS contains -fno-lto') + enable_lto = False ldflags = [] if 'LDFLAGS' in os.environ: ldflags.append(os.environ['LDFLAGS']) + if enable_lto is False: + return env + if has_flag(self.compiler, '-fvisibility=hidden'): for ext in self.extensions: ext.extra_compile_args.append('-fvisibility=hidden') From a786d2a3e09b4853aaa7d5d60089a6e0dbc8b444 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 27 Jan 2021 17:08:45 -0500 Subject: [PATCH 2/4] Inline convert_voidptr to tkagg. It's only used in this file, and pulling in the whole `py_converters.h` causes this extension to unnecessarily require compiling against NumPy. --- setupext.py | 1 - src/_tkagg.cpp | 11 +++++++---- src/py_converters.cpp | 7 ------- src/py_converters.h | 1 - 4 files changed, 7 insertions(+), 13 deletions(-) diff --git a/setupext.py b/setupext.py index 95f946752fcf..619f86e385a5 100644 --- a/setupext.py +++ b/setupext.py @@ -447,7 +447,6 @@ def get_extensions(self): ext = Extension( "matplotlib.backends._tkagg", [ "src/_tkagg.cpp", - "src/py_converters.cpp", ], include_dirs=["src"], # psapi library needed for finding Tcl/Tk at run time. diff --git a/src/_tkagg.cpp b/src/_tkagg.cpp index 3e5793f03ce8..5f058d14e0f0 100644 --- a/src/_tkagg.cpp +++ b/src/_tkagg.cpp @@ -33,14 +33,17 @@ #define dlsym GetProcAddress #else #include -// Suppress -Wunused-function on POSIX, but not on Windows where that would -// lead to PY_ARRAY_UNIQUE_SYMBOL being an unresolved external. -#define NO_IMPORT_ARRAY #endif // Include our own excerpts from the Tcl / Tk headers #include "_tkmini.h" -#include "py_converters.h" + +static int convert_voidptr(PyObject *obj, void *p) +{ + void **val = (void **)p; + *val = PyLong_AsVoidPtr(obj); + return *val != NULL ? 1 : !PyErr_Occurred(); +} // Global vars for Tk functions. We load these symbols from the tkinter // extension module or loaded Tk libraries at run-time. diff --git a/src/py_converters.cpp b/src/py_converters.cpp index ace8332dda76..a4c0b1909940 100644 --- a/src/py_converters.cpp +++ b/src/py_converters.cpp @@ -94,13 +94,6 @@ int convert_from_attr(PyObject *obj, const char *name, converter func, void *p) return 1; } -int convert_voidptr(PyObject *obj, void *p) -{ - void **val = (void **)p; - *val = PyLong_AsVoidPtr(obj); - return *val != NULL ? 1 : !PyErr_Occurred(); -} - int convert_double(PyObject *obj, void *p) { double *val = (double *)p; diff --git a/src/py_converters.h b/src/py_converters.h index 33af43a474e8..2c19acdaaf80 100644 --- a/src/py_converters.h +++ b/src/py_converters.h @@ -22,7 +22,6 @@ typedef int (*converter)(PyObject *, void *); int convert_from_attr(PyObject *obj, const char *name, converter func, void *p); int convert_from_method(PyObject *obj, const char *name, converter func, void *p); -int convert_voidptr(PyObject *obj, void *p); int convert_double(PyObject *obj, void *p); int convert_bool(PyObject *obj, void *p); int convert_cap(PyObject *capobj, void *capp); From 8bb0379feb00ae6f00530eec2643f5752a3c89b0 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Thu, 14 Jan 2021 00:45:20 -0500 Subject: [PATCH 3/4] Disable LTO in one test configuration. --- .github/workflows/tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 87b7f90f6032..0da4b5df4993 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -32,6 +32,7 @@ jobs: python-version: 3.7 extra-requirements: '-r requirements/testing/travis_extra.txt' XVFB_RUN: xvfb-run -a + CFLAGS: "-fno-lto" # Ensure that disabling LTO works. - os: ubuntu-16.04 python-version: 3.8 extra-requirements: '-r requirements/testing/travis_extra.txt' From 988bfc597c96bcf7f5be802df4c7c8ed39e85e06 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Tue, 26 Jan 2021 23:53:05 -0500 Subject: [PATCH 4/4] Reduce redundancy while checking *FLAGS for LTO. --- setup.py | 45 ++++++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/setup.py b/setup.py index f4af218f100b..6043b03f025b 100644 --- a/setup.py +++ b/setup.py @@ -111,31 +111,26 @@ def add_optimization_flags(self): enable_lto = setupext.config.getboolean('libs', 'enable_lto', fallback=None) - if 'CFLAGS' in os.environ: - if '-fno-lto' in os.environ['CFLAGS']: - if enable_lto is True: - raise ValueError('Configuration enable_lto=True, but ' - 'CFLAGS contains -fno-lto') - enable_lto = False - cppflags = [] - if 'CPPFLAGS' in os.environ: - cppflags.append(os.environ['CPPFLAGS']) - if '-fno-lto' in os.environ['CPPFLAGS']: - if enable_lto is True: - raise ValueError('Configuration enable_lto=True, but ' - 'CPPFLAGS contains -fno-lto') - enable_lto = False - cxxflags = [] - if 'CXXFLAGS' in os.environ: - cxxflags.append(os.environ['CXXFLAGS']) - if '-fno-lto' in os.environ['CXXFLAGS']: - if enable_lto is True: - raise ValueError('Configuration enable_lto=True, but ' - 'CXXFLAGS contains -fno-lto') - enable_lto = False - ldflags = [] - if 'LDFLAGS' in os.environ: - ldflags.append(os.environ['LDFLAGS']) + def prepare_flags(name, enable_lto): + """ + Prepare *FLAGS from the environment. + + If set, return them, and also check whether LTO is disabled in each + one, raising an error if Matplotlib config explicitly enabled LTO. + """ + if name in os.environ: + if '-fno-lto' in os.environ[name]: + if enable_lto is True: + raise ValueError('Configuration enable_lto=True, but ' + '{0} contains -fno-lto'.format(name)) + enable_lto = False + return [os.environ[name]], enable_lto + return [], enable_lto + + _, enable_lto = prepare_flags('CFLAGS', enable_lto) # Only check lto. + cppflags, enable_lto = prepare_flags('CPPFLAGS', enable_lto) + cxxflags, enable_lto = prepare_flags('CXXFLAGS', enable_lto) + ldflags, enable_lto = prepare_flags('LDFLAGS', enable_lto) if enable_lto is False: return env