[Mesa-stable] [PATCH v4] i965: compiler option msse2 and mstackrealign

[Dylan Baker]

Quoting Sergii Romantsov (2018-09-05 04:40:37)
> Seems in case of 32-bit library, usage of msse2 makes
> some stack corruption or incorrect instructions.
> Usage with mstackrealign fixes that case.
> 
> v2: Fixed meson.
> 
> v3: Definition of c_sse2_args moved on the top (L.Landwerlin).
>     Added mstackrealign for Android's mks where msee4.1 is used.
> 
> v4: Added for Vulkan also.
> 
> CC: <mesa-stable at lists.freedesktop.org>
> Fixes: 6b05c080f202 (i965: Compile with -msse3)
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107779
> Signed-off-by: Sergii Romantsov <sergii.romantsov at globallogic.com>
> ---
>  src/intel/Makefile.vulkan.am          | 2 +-
>  src/intel/meson.build                 | 1 +
>  src/intel/vulkan/meson.build          | 4 ++--
>  src/mesa/Android.libmesa_dricore.mk   | 2 +-
>  src/mesa/Android.libmesa_sse41.mk     | 2 +-
>  src/mesa/drivers/dri/i965/Makefile.am | 2 +-
>  src/mesa/drivers/dri/i965/meson.build | 6 +++---
>  7 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/src/intel/Makefile.vulkan.am b/src/intel/Makefile.vulkan.am
> index 9555d98..d511263 100644
> --- a/src/intel/Makefile.vulkan.am
> +++ b/src/intel/Makefile.vulkan.am
> @@ -104,7 +104,7 @@ noinst_LTLIBRARIES += $(VULKAN_PER_GEN_LIBS)
>  
>  VULKAN_CFLAGS = \
>         $(AM_CFLAGS) \
> -       -msse2
> +       -msse2 -mstackrealign
>  
>  VULKAN_CPPFLAGS = \
>         -I$(top_srcdir)/src/compiler \
> diff --git a/src/intel/meson.build b/src/intel/meson.build
> index b3dcbdc..3c57e79 100644
> --- a/src/intel/meson.build
> +++ b/src/intel/meson.build
> @@ -18,6 +18,7 @@
>  # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>  # SOFTWARE.
>  
> +c_sse2_args = ['-msse2', '-mstackrealign']
>  inc_intel = include_directories('.')
>  
>  subdir('blorp')
> diff --git a/src/intel/vulkan/meson.build b/src/intel/vulkan/meson.build
> index e11bcb0..f1beb1d 100644
> --- a/src/intel/vulkan/meson.build
> +++ b/src/intel/vulkan/meson.build
> @@ -102,7 +102,7 @@ foreach g : [['70', ['gen7_cmd_buffer.c']], ['75', ['gen7_cmd_buffer.c']],
>        inc_vulkan_wsi,
>      ],
>      c_args : [
> -      c_vis_args, no_override_init_args, '-msse2',
> +      c_vis_args, no_override_init_args, c_sse2_args,
>        '-DGEN_VERSIONx10=@0@'.format(_gen),
>      ],
>      dependencies : [dep_libdrm, dep_valgrind, idep_nir_headers],
> @@ -146,7 +146,7 @@ anv_deps = [
>  anv_flags = [
>    c_vis_args,
>    no_override_init_args,
> -  '-msse2',
> +  c_sse2_args,
>  ]
>  
>  if with_platform_x11
> diff --git a/src/mesa/Android.libmesa_dricore.mk b/src/mesa/Android.libmesa_dricore.mk
> index 34fd858..7921177 100644
> --- a/src/mesa/Android.libmesa_dricore.mk
> +++ b/src/mesa/Android.libmesa_dricore.mk
> @@ -49,7 +49,7 @@ ifeq ($(ARCH_X86_HAVE_SSE4_1),true)
>  LOCAL_WHOLE_STATIC_LIBRARIES := \
>         libmesa_sse41
>  LOCAL_CFLAGS := \
> -       -msse4.1 \
> +       -msse4.1 -mstackrealign \
>         -DUSE_SSE41
>  endif
>  
> diff --git a/src/mesa/Android.libmesa_sse41.mk b/src/mesa/Android.libmesa_sse41.mk
> index da40f43..de19a1f 100644
> --- a/src/mesa/Android.libmesa_sse41.mk
> +++ b/src/mesa/Android.libmesa_sse41.mk
> @@ -34,7 +34,7 @@ LOCAL_SRC_FILES += \
>         $(X86_SSE41_FILES)
>  
>  LOCAL_CFLAGS := \
> -       -msse4.1
> +       -msse4.1 -mstackrealign
>  
>  LOCAL_C_INCLUDES := \
>         $(MESA_TOP)/src/mapi \
> diff --git a/src/mesa/drivers/dri/i965/Makefile.am b/src/mesa/drivers/dri/i965/Makefile.am
> index 889d4c6..0afa7a2 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.am
> +++ b/src/mesa/drivers/dri/i965/Makefile.am
> @@ -44,7 +44,7 @@ AM_CFLAGS = \
>         $(WNO_OVERRIDE_INIT) \
>         $(LIBDRM_CFLAGS) \
>         $(VALGRIND_CFLAGS) \
> -       -msse2
> +       -msse2 -mstackrealign
>  
>  AM_CXXFLAGS = $(AM_CFLAGS)
>  
> diff --git a/src/mesa/drivers/dri/i965/meson.build b/src/mesa/drivers/dri/i965/meson.build
> index 6c94f4a..b95e2d7 100644
> --- a/src/mesa/drivers/dri/i965/meson.build
> +++ b/src/mesa/drivers/dri/i965/meson.build
> @@ -142,7 +142,7 @@ foreach v : ['40', '45', '50', '60', '70', '75', '80', '90', '100', '110']
>      ['genX_blorp_exec.c', 'genX_state_upload.c', gen_xml_pack],
>      include_directories : [inc_common, inc_intel, inc_dri_common],
>      c_args : [
> -      c_vis_args, no_override_init_args, '-msse2',
> +      c_vis_args, no_override_init_args, c_sse2_args,
>        '-DGEN_VERSIONx10=@0@'.format(v),
>      ],
>      dependencies : [dep_libdrm, idep_nir_headers],
> @@ -183,8 +183,8 @@ libi965 = static_library(
>    include_directories : [
>      inc_common, inc_intel, inc_dri_common, inc_util, inc_drm_uapi,
>    ],
> -  c_args : [c_vis_args, no_override_init_args, '-msse2'],
> -  cpp_args : [cpp_vis_args, '-msse2'],
> +  c_args : [c_vis_args, no_override_init_args, c_sse2_args],
> +  cpp_args : [cpp_vis_args, c_sse2_args],
>    link_with : [
>      i965_gen_libs, libintel_common, libintel_dev, libisl, libintel_compiler,
>      libblorp,
> -- 
> 2.7.4
> 

Looks good to me, thanks for following up on this. Just one very tiny nit, can
you replace i965 with intel in the title of the commit, since we're touch vulkan
as well?

With that,
Reviewed-by: Dylan Baker <dylan at pnwbakers.com>

I assume you have commit access?

Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-stable/attachments/20180905/e8fa6541/attachment-0001.sig>