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-sta...@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.romant...@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 <dy...@pnwbakers.com>

I assume you have commit access?

Dylan

Attachment: signature.asc
Description: signature

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

Reply via email to