[Mesa-dev] [PATCH v3 1/2] configure+mesa/st: unify check for -std=c++11 support and enable accordingly

[Gert Wollny]

Am Montag, den 02.10.2017, 13:37 +0100 schrieb Emil Velikov:
> Hi Gert,
> 
> On 19 September 2017 at 12:35, Gert Wollny <gw.fossdev at gmail.com>
> wrote:
> > Unify the CXX feature tests for C++11 support that is required for
> > SWR, clover, and mesa/st/tests.
> > 
> 
> Surely we'll have capable compiler in 90+% of the time, still asking
> for C++11 for single test is an overkill.

Hence my attempt to unify the test for the three cases where c++11 is
already mandatory. 

[...]

> 
> > ---
> >  .travis.yml                                   |   1 +
> 
> Unrelated/undocumented change?

Sorry. slipped my attention. 

> 
> >  configure.ac                                  |  13 +-
> >  m4/ax_cxx_compile_stdcxx.m4                   | 987
> > ++++++++++++++++++++++++++
> 
> Please don't add custom m4 unless absolutely needed.

The advantage of this module is that it actually tests features of the
required standard, and if the compiler enables them by default, no flag
is needed. The result is cached and the test already support c++14 and
c++17. 

I can also use the already established AX_CHECK_COMPILE_FLAG, and if
you say so, I'll do it, but IMHO the required changes to configure.ac
make the code look less clean. 

> 
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -301,8 +301,12 @@ AX_CHECK_COMPILE_FLAG([-
> > Wall],                                 [CXXFLAGS="$CXXFL
> >  AX_CHECK_COMPILE_FLAG([-fno-math-
> > errno],                       [CXXFLAGS="$CXXFLAGS -fno-math-
> > errno"])
> >  AX_CHECK_COMPILE_FLAG([-fno-trapping-
> > math],                    [CXXFLAGS="$CXXFLAGS -fno-trapping-
> > math"])
> >  AX_CHECK_COMPILE_FLAG([-
> > fvisibility=hidden],                   [VISIBILITY_CXXFLAGS="-
> > fvisibility=hidden"])
> > +AX_CXX_COMPILE_STDCXX(11, noext, optional)
> > +
> >  AC_LANG_POP([C++])
> > 
> > +AM_CONDITIONAL(HAVE_STD_CXX11, test "x$HAVE_CXX11" = "x1")
> > +
> >  # Flags to help ensure that certain portions of the code -- and
> > only those
> >  # portions -- can be built with MSVC:
> >  # - src/util, src/gallium/auxiliary, rc/gallium/drivers/llvmpipe,
> > and
> > @@ -2234,9 +2238,7 @@ if test "x$enable_opencl" = xyes; then
> >          AC_MSG_ERROR([cannot enable OpenCL without Gallium])
> >      fi
> > 
> > -    if test $GCC_VERSION_MAJOR -lt 4 -o $GCC_VERSION_MAJOR -eq 4
> > -a $GCC_VERSION_MINOR -lt 7; then
> > -        AC_MSG_ERROR([gcc >= 4.7 is required to build clover])
> > -    fi
> > +    AX_CXX_COMPILE_STDCXX(11, noext, mandatory)
> > 
> 
> Why check again - you already know the result.

The result is cached, so calling the macro again is of no run-time
consequence. The alternative is to replace this one-liner with a three
line if-then statement (same if I switch to AX_CHECK_COMPILE_FLAG). 

> 
> >      if test "x$have_libclc" = xno; then
> >          AC_MSG_ERROR([pkg-config cannot find libclc.pc which is
> > required to build clover.
> > @@ -2518,10 +2520,7 @@ if test -n "$with_gallium_drivers"; then
> >          xswr)
> >              llvm_require_version $LLVM_REQUIRED_SWR "swr"
> > 
> > -            swr_require_cxx_feature_flags "C++11" "__cplusplus >=
> > 201103L" \
> > -                ",-std=c++11" \
> > -                SWR_CXX11_CXXFLAGS
> > -            AC_SUBST([SWR_CXX11_CXXFLAGS])
> > +           AX_CXX_COMPILE_STDCXX(11, noext, mandatory)
> > 
> 
> Ditto.

As commented above. 


> 
> You'd want to update the other SWR_CXX11_CXXFLAGS reference further
> down.
I'll re-check, but I though I got all the references (and on travis all
compiled fine). 

> 
> > --- a/src/gallium/drivers/swr/Makefile.am
> > +++ b/src/gallium/drivers/swr/Makefile.am
> > @@ -22,7 +22,7 @@
> >  include Makefile.sources
> >  include $(top_srcdir)/src/gallium/Automake.inc
> > 
> > -AM_CXXFLAGS = $(GALLIUM_DRIVER_CFLAGS) $(SWR_CXX11_CXXFLAGS)
> > +AM_CXXFLAGS = $(GALLIUM_DRIVER_CFLAGS) $(CXX11_FLAGS)
> > 
> >  noinst_LTLIBRARIES = libmesaswr.la
> > 
> > @@ -39,7 +39,7 @@ COMMON_CXXFLAGS = \
> >         -fno-strict-aliasing \
> >         $(GALLIUM_DRIVER_CFLAGS) \
> >         $(LLVM_CXXFLAGS) \
> > -       $(SWR_CXX11_CXXFLAGS) \
> > +       $(CXX11_FLAGS) \
> >         -I$(builddir)/rasterizer/codegen \
> >         -I$(builddir)/rasterizer/core \
> >         -I$(builddir)/rasterizer/jitter \
> > diff --git a/src/gallium/state_trackers/clover/Makefile.am
> > b/src/gallium/state_trackers/clover/Makefile.am
> > index 321393536d..35b43b380c 100644
> > --- a/src/gallium/state_trackers/clover/Makefile.am
> > +++ b/src/gallium/state_trackers/clover/Makefile.am
> > @@ -31,14 +31,14 @@ endif
> >  noinst_LTLIBRARIES = libclover.la libcltgsi.la libclllvm.la
> > 
> >  libcltgsi_la_CXXFLAGS = \
> > -       -std=c++11 \
> > +       ${CXX11_FLAGS} \
> 
> Please stay consistent through the file 's/{/(';'s/}/)'

Okay, I'll recheck consistency. 

many thanks, 
Gert