On 27/02/15 15:59, Ian Romanick wrote:
I like the idea as it should prevent future thrash.  There are a couple
comments below.

On 02/26/2015 08:51 AM, Jose Fonseca wrote:
The main objective of this change is to enable Linux developers to use
more of C99 throughout Mesa, with confidence that the portions that need
to be built with MSVC -- and only those portions --, stay portable.

This is achieved by using the appropriate -Werror= options only on the
places they need to be used.

Unfortunately we still need MSVC 2008 on a few portions of the code
(namely llvmpipe and its dependencies).  I hope to eventually eliminate
this so that we can use C99 everywhere, but there are technical/logistic
challenges (specifically, newer Windows SDKs no longer bundle MSVC,
instead require a full installation of Visual Studio, and that has
hindered adoption of newer MSVC versions on our build processes.)
Thankfully we have more directy control over our OpenGL driver, which is
why we're now able to migrate to MSVC 2013 for most of the tree.
---
  configure.ac                               | 17 +++++++++++++++++
  src/egl/main/Makefile.am                   |  1 +
  src/gallium/auxiliary/Makefile.am          |  7 +++++--
  src/gallium/drivers/llvmpipe/Makefile.am   |  6 ++++--
  src/gallium/state_trackers/egl/Makefile.am |  3 ++-
  src/gallium/targets/egl-static/Makefile.am |  3 ++-
  src/glsl/Makefile.am                       |  8 ++++++--
  src/loader/Makefile.am                     |  1 +
  src/mapi/Makefile.am                       |  4 +++-
  src/mesa/Makefile.am                       | 10 ++++++++--
  src/util/Makefile.am                       |  3 ++-
  11 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5fbb7bc..22dc023 100644
--- a/configure.ac
+++ b/configure.ac
@@ -263,6 +263,18 @@ if test "x$GCC" = xyes; then
      # gcc's builtin memcmp is slower than glibc's
      # 
https://urldefense.proofpoint.com/v2/url?u=http-3A__gcc.gnu.org_bugzilla_show-5Fbug.cgi-3Fid-3D43052&d=AwICaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=gzXuw5K38HQj__hbWmSmucwxyJyOitvZ880mS5zyZ-w&s=KMhA_Gf3BUCyos0GcxsRFjkuWQogkFDUojBIet4t1o4&e=
      CFLAGS="$CFLAGS -fno-builtin-memcmp"
+
+    # Flags to help ensure that certain portions of the code -- and only those
+    # portions -- can be built with MSVC:
+    # - src/util, src/gallium/auxiliary, and src/gallium/drivers/llvmpipe needs
+    #   to build with Windows SDK 7.0.7600, which bundles MSVC 2008
+    # - non-Linux/Posix OpenGL portions needs to build on MSVC 2013 (which
+    #   supports most of C99)
+    # - the rest has no compiler compiler restrictions
+    MSVC2013_COMPAT_CFLAGS="-Werror=vla -Werror=pointer-arith"
+    MSVC2013_COMPAT_CXXFLAGS="-Werror=vla -Werror=pointer-arith"
+    MSVC2008_COMPAT_CFLAGS="$MSVC2013_COMPAT_CFLAGS 
-Werror=declaration-after-statement"
+    MSVC2008_COMPAT_CXXFLAGS="$MSVC2013_COMPAT_CXXFLAGS"
  fi
  if test "x$GXX" = xyes; then
      CXXFLAGS="$CXXFLAGS -Wall"
@@ -288,6 +300,11 @@ if test "x$GXX" = xyes; then
      CXXFLAGS="$CXXFLAGS -fno-builtin-memcmp"
  fi

+AC_SUBST([MSVC2013_COMPAT_CFLAGS])
+AC_SUBST([MSVC2013_COMPAT_CXXFLAGS])
+AC_SUBST([MSVC2008_COMPAT_CFLAGS])
+AC_SUBST([MSVC2008_COMPAT_CXXFLAGS])
+
  dnl even if the compiler appears to support it, using visibility attributes 
isn't
  dnl going to do anything useful currently on cygwin apart from emit lots of 
warnings
  case "$host_os" in
diff --git a/src/egl/main/Makefile.am b/src/egl/main/Makefile.am
index d21d8a9..a4db210 100644
--- a/src/egl/main/Makefile.am
+++ b/src/egl/main/Makefile.am
@@ -26,6 +26,7 @@ AM_CFLAGS = \
        -I$(top_srcdir)/src/gbm/main \
        $(DEFINES) \
        $(VISIBILITY_CFLAGS) \
+       $(MSVC2013_COMPAT_CFLAGS) \
        $(EGL_CFLAGS) \
        -D_EGL_NATIVE_PLATFORM=$(EGL_NATIVE_PLATFORM) \
        -D_EGL_DRIVER_SEARCH_DIR=\"$(libdir)/egl\" \
diff --git a/src/gallium/auxiliary/Makefile.am 
b/src/gallium/auxiliary/Makefile.am
index 4b62057..27a8b3f 100644
--- a/src/gallium/auxiliary/Makefile.am
+++ b/src/gallium/auxiliary/Makefile.am
@@ -12,9 +12,12 @@ noinst_LTLIBRARIES = libgallium.la
  AM_CFLAGS = \
        -I$(top_srcdir)/src/gallium/auxiliary/util \
        $(GALLIUM_CFLAGS) \
-       $(VISIBILITY_CFLAGS)
+       $(VISIBILITY_CFLAGS) \
+       $(MSVC2008_COMPAT_CXXFLAGS)

-AM_CXXFLAGS = $(VISIBILITY_CXXFLAGS)
+AM_CXXFLAGS = \
+       $(VISIBILITY_CXXFLAGS) \
+       $(MSVC2008_COMPAT_CXXFLAGS)

  libgallium_la_SOURCES = \
        $(C_SOURCES) \
diff --git a/src/gallium/drivers/llvmpipe/Makefile.am 
b/src/gallium/drivers/llvmpipe/Makefile.am
index 0bd4282..1d3853e 100644
--- a/src/gallium/drivers/llvmpipe/Makefile.am
+++ b/src/gallium/drivers/llvmpipe/Makefile.am
@@ -25,10 +25,12 @@ include $(top_srcdir)/src/gallium/Automake.inc

  AM_CFLAGS = \
        $(GALLIUM_DRIVER_CFLAGS) \
-       $(LLVM_CFLAGS)
+       $(LLVM_CFLAGS) \
+       $(MSVC2008_COMPAT_CFLAGS)
  AM_CXXFLAGS= \
        $(GALLIUM_DRIVER_CXXFLAGS) \
-       $(LLVM_CXXFLAGS)
+       $(LLVM_CXXFLAGS) \
+       $(MSVC2008_COMPAT_CXXFLAGS)

  noinst_LTLIBRARIES = libllvmpipe.la

diff --git a/src/gallium/state_trackers/egl/Makefile.am 
b/src/gallium/state_trackers/egl/Makefile.am
index 31546a7..f13fcb2 100644
--- a/src/gallium/state_trackers/egl/Makefile.am
+++ b/src/gallium/state_trackers/egl/Makefile.am
@@ -27,7 +27,8 @@ include $(top_srcdir)/src/gallium/Automake.inc

  AM_CFLAGS = \
        $(GALLIUM_CFLAGS) \
-       $(VISIBILITY_CFLAGS)
+       $(VISIBILITY_CFLAGS) \
+       $(MSVC2013_COMPAT_CFLAGS)

  AM_CPPFLAGS = \
        -I$(top_srcdir)/src/egl/main \
diff --git a/src/gallium/targets/egl-static/Makefile.am 
b/src/gallium/targets/egl-static/Makefile.am
index 85f2ac1..51c3f05 100644
--- a/src/gallium/targets/egl-static/Makefile.am
+++ b/src/gallium/targets/egl-static/Makefile.am
@@ -31,7 +31,8 @@
  include $(top_srcdir)/src/gallium/Automake.inc

  AM_CFLAGS = \
-       $(GALLIUM_TARGET_CFLAGS)
+       $(GALLIUM_TARGET_CFLAGS) \
+       $(MSVC2013_COMPAT_CFLAGS)

  AM_CPPFLAGS = \
        -I$(top_srcdir)/src/gallium/state_trackers/egl \
diff --git a/src/glsl/Makefile.am b/src/glsl/Makefile.am
index 5a0a643..b466a3b 100644
--- a/src/glsl/Makefile.am
+++ b/src/glsl/Makefile.am
@@ -33,8 +33,12 @@ AM_CPPFLAGS = \
        -I$(top_srcdir)/src/gtest/include \
        -I$(top_builddir)/src/glsl/nir \
        $(DEFINES)
-AM_CFLAGS = $(VISIBILITY_CFLAGS)
-AM_CXXFLAGS = $(VISIBILITY_CXXFLAGS)
+AM_CFLAGS = \
+       $(VISIBILITY_CFLAGS) \
+       $(MSVC2013_COMPAT_CFLAGS)
+AM_CXXFLAGS = \
+       $(VISIBILITY_CXXFLAGS) \
+       $(MSVC2013_COMPAT_CXXFLAGS)

  EXTRA_DIST = tests glcpp/tests README TODO glcpp/README       \
        glsl_lexer.ll                                   \
diff --git a/src/loader/Makefile.am b/src/loader/Makefile.am
index 36ddba8..3d32279 100644
--- a/src/loader/Makefile.am
+++ b/src/loader/Makefile.am
@@ -30,6 +30,7 @@ libloader_la_CPPFLAGS = \
        -I$(top_srcdir)/include \
        -I$(top_srcdir)/src \
        $(VISIBILITY_CFLAGS) \
+       $(MSVC2013_COMPAT_CFLAGS) \
        $(LIBUDEV_CFLAGS)

I don't think this is necessary for src/loader.  Isn't that only built
for DRI?

No, it's being built with MSVC too, and unconditionally.

But I think that after we remove src/gallium/state_trackers/egl and src/gallium/targets/egl-static then we could stop building it too.


  libloader_la_SOURCES = $(LOADER_C_FILES)
diff --git a/src/mapi/Makefile.am b/src/mapi/Makefile.am
index 6794682..b0a6c8c 100644
--- a/src/mapi/Makefile.am
+++ b/src/mapi/Makefile.am
@@ -39,7 +39,9 @@ EXTRA_DIST = \
        glapi/SConscript \
        shared-glapi/SConscript

-AM_CFLAGS = $(PTHREAD_CFLAGS)
+AM_CFLAGS = \
+       $(PTHREAD_CFLAGS) \
+       $(MSVC2013_COMPAT_CFLAGS)
  AM_CPPFLAGS =                                                 \
        $(DEFINES)                                              \
        $(SELINUX_CFLAGS)                                       \
diff --git a/src/mesa/Makefile.am b/src/mesa/Makefile.am
index b6cb8f1..5e9a820 100644
--- a/src/mesa/Makefile.am
+++ b/src/mesa/Makefile.am
@@ -136,8 +136,14 @@ noinst_LTLIBRARIES += libmesagallium.la
  endif

  AM_CPPFLAGS = $(DEFINES) $(INCLUDE_DIRS)
-AM_CFLAGS = $(LLVM_CFLAGS) $(VISIBILITY_CFLAGS)
-AM_CXXFLAGS = $(LLVM_CFLAGS) $(VISIBILITY_CXXFLAGS)
+AM_CFLAGS = \
+       $(LLVM_CFLAGS) \
+       $(VISIBILITY_CFLAGS) \
+       $(MSVC2013_COMPAT_CFLAGS)
+AM_CXXFLAGS = \
+       $(LLVM_CFLAGS) \
+       $(VISIBILITY_CXXFLAGS) \
+       $(MSVC2013_COMPAT_CXXFLAGS)

Forgive my ignorance about the build system... does applying this here
affect building files down in src/mesa/drivers/dri?

I _think_ it applies to the current dir.

For e.g., src/mesa/drivers/dri/swrast/Makefile.am defines AM_CFLAGS from scratch.

But I confess I'm not that familiar with autotools neither, so I'm not sure the exact semantics, i.e., whether AM_*FLAGS _need_ to be defined on every directory, or merely that we happen to overwrite AM_*FLAGS on most directories...




  ARCH_LIBS =

diff --git a/src/util/Makefile.am b/src/util/Makefile.am
index 9af2330..29b66e7 100644
--- a/src/util/Makefile.am
+++ b/src/util/Makefile.am
@@ -34,7 +34,8 @@ libmesautil_la_CPPFLAGS = \
        -I$(top_srcdir)/src/gallium/include \
        -I$(top_srcdir)/src/gallium/auxiliary \
        $(SHA1_CFLAGS) \
-       $(VISIBILITY_CFLAGS)
+       $(VISIBILITY_CFLAGS) \
+       $(MSVC2008_COMPAT_CFLAGS)

  libmesautil_la_SOURCES = \
        $(MESA_UTIL_FILES) \


Jose
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to