Re: [Mesa-dev] [PATCH v1] i965: compiler option msse2 and mstackrealign

2018-09-05 Thread Lionel Landwerlin 

On 05/09/2018 10:55, Sergii Romantsov wrote:

Hello, sorry for delay in answer.

I think you might need to update the other build systems
(Android.mk/meson)

Have done it for meson, but with Android i can't check if issue 
present or if it will be fixed. And seems it requires some extra 
knowledge/time. Could we try to discuss it in separate thread/patch?



Actually, it looks like Android.mk sources its flags from the 
Makefile.common.mk so this is probably fine.





The toplevel meson/makefile have a -mstackrealign that seem to
make the description of the issue you're fixing.
Maybe that's where this change should go.

Do you mean to move compilation flag for all libs? Seems at this 
moment flag mstackrealign is needed only for i965-dri, so maybe its 
better to enable only if needed?



Sounds like Dylan knows better. Adding it to the src/intel level is fine.




For meson it might be nice to add a c_sse2_args = ['-msse2',
'-mstackrealign'] at the src/intel level 


Done

Also, since this is extremely trivial and fixes bugs, this should
be cc'd to mesa stable for 18.1 and 18.2

Sorry, not clear for me: should i do anything with that?



Just add following tag, so that people maintaining the stable branches 
know what to cherry-pick from master :



CC: 





On Wed, Sep 5, 2018 at 1:19 AM, Dylan Baker > wrote:


Quoting Sergii Romantsov (2018-09-04 03:46:23)
> Seems in case of 32-bit library, usage of msse2 makes
> some stack corruption or incorrect instructions.
> Usage with mstackrealign fixes that case.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107779

> Signed-off-by: Sergii Romantsov
mailto:sergii.romant...@globallogic.com>>
> ---
>  src/mesa/drivers/dri/i965/Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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)
>
> --
> 2.7.4
>

Also, since this is extremely trivial and fixes bugs, this should
be cc'd to
mesa stable for 18.1 and 18.2

Thanks,
Dylan

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





--
Sergii Romantsov
GlobalLogic Inc.
www.globallogic.com 

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



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

Re: [Mesa-dev] [PATCH v1] i965: compiler option msse2 and mstackrealign

2018-09-05 Thread Sergii Romantsov 
Hello, sorry for delay in answer.

I think you might need to update the other build systems (Android.mk/meson)

Have done it for meson, but with Android i can't check if issue present or
if it will be fixed. And seems it requires some extra knowledge/time. Could
we try to discuss it in separate thread/patch?

The toplevel meson/makefile have a -mstackrealign that seem to make the
> description of the issue you're fixing.
> Maybe that's where this change should go.

Do you mean to move compilation flag for all libs? Seems at this moment
flag mstackrealign is needed only for i965-dri, so maybe its better to
enable only if needed?

For meson it might be nice to add a c_sse2_args = ['-msse2',
> '-mstackrealign'] at the src/intel level

Done

Also, since this is extremely trivial and fixes bugs, this should be cc'd
> to mesa stable for 18.1 and 18.2

Sorry, not clear for me: should i do anything with that?


On Wed, Sep 5, 2018 at 1:19 AM, Dylan Baker  wrote:

> Quoting Sergii Romantsov (2018-09-04 03:46:23)
> > Seems in case of 32-bit library, usage of msse2 makes
> > some stack corruption or incorrect instructions.
> > Usage with mstackrealign fixes that case.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107779
> > Signed-off-by: Sergii Romantsov 
> > ---
> >  src/mesa/drivers/dri/i965/Makefile.am | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > 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)
> >
> > --
> > 2.7.4
> >
>
> Also, since this is extremely trivial and fixes bugs, this should be cc'd
> to
> mesa stable for 18.1 and 18.2
>
> Thanks,
> Dylan
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>


-- 
Sergii Romantsov
GlobalLogic Inc.
www.globallogic.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1] i965: compiler option msse2 and mstackrealign

2018-09-04 Thread Dylan Baker 
Quoting Sergii Romantsov (2018-09-04 03:46:23)
> Seems in case of 32-bit library, usage of msse2 makes
> some stack corruption or incorrect instructions.
> Usage with mstackrealign fixes that case.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107779
> Signed-off-by: Sergii Romantsov 
> ---
>  src/mesa/drivers/dri/i965/Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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)
>  
> -- 
> 2.7.4
> 

Also, since this is extremely trivial and fixes bugs, this should be cc'd to
mesa stable for 18.1 and 18.2

Thanks,
Dylan


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1] i965: compiler option msse2 and mstackrealign

2018-09-04 Thread Dylan Baker 
Quoting Lionel Landwerlin (2018-09-04 05:22:31)
> On 04/09/2018 11:46, Sergii Romantsov wrote:
> > Seems in case of 32-bit library, usage of msse2 makes
> > some stack corruption or incorrect instructions.
> > Usage with mstackrealign fixes that case.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107779
> > Signed-off-by: Sergii Romantsov 
> > ---
> >   src/mesa/drivers/dri/i965/Makefile.am | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > 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)
> >   
> 
> Hey Sergii,
> 
> Thanks a lot for looking into this issue.
> I think you might need to update the other build systems (Android.mk/meson).
> 
> The toplevel meson/makefile have a -mstackrealign that seem to make the 
> description of the issue you're fixing.
> Maybe that's where this change should go.
> 
> Thanks,
> 
> -
> Lionel

There's a similar usage in anv that should be fixed.

My reading of the gcc man page and Matt's understanding is that -mstackrealign
is a no-op on x86_64, so just putting it in there should be fine. For meson it
might be nice to add a c_sse2_args = ['-msse2', '-mstackrealign'] at the
src/intel level and use instead of adding -mstackrealign everywhere, but we can
also do that as a follow-up.

Dylan


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1] i965: compiler option msse2 and mstackrealign

2018-09-04 Thread Kenneth Graunke 
On Tuesday, September 4, 2018 5:22:31 AM PDT Lionel Landwerlin wrote:
> On 04/09/2018 11:46, Sergii Romantsov wrote:
> > Seems in case of 32-bit library, usage of msse2 makes
> > some stack corruption or incorrect instructions.
> > Usage with mstackrealign fixes that case.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107779
> > Signed-off-by: Sergii Romantsov 
> > ---
> >   src/mesa/drivers/dri/i965/Makefile.am | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > 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)
> >   
> 
> Hey Sergii,
> 
> Thanks a lot for looking into this issue.
> I think you might need to update the other build systems (Android.mk/meson).
> 
> The toplevel meson/makefile have a -mstackrealign that seem to make the 
> description of the issue you're fixing.
> Maybe that's where this change should go.
> 
> Thanks,
> 
> -
> Lionel
> 
> 

This may also fix:
https://bugs.freedesktop.org/show_bug.cgi?id=98584


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v1] i965: compiler option msse2 and mstackrealign

2018-09-04 Thread Lionel Landwerlin 

On 04/09/2018 11:46, Sergii Romantsov wrote:

Seems in case of 32-bit library, usage of msse2 makes
some stack corruption or incorrect instructions.
Usage with mstackrealign fixes that case.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107779
Signed-off-by: Sergii Romantsov 
---
  src/mesa/drivers/dri/i965/Makefile.am | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

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)
  


Hey Sergii,

Thanks a lot for looking into this issue.
I think you might need to update the other build systems (Android.mk/meson).

The toplevel meson/makefile have a -mstackrealign that seem to make the 
description of the issue you're fixing.

Maybe that's where this change should go.

Thanks,

-
Lionel

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

[Mesa-dev] [PATCH v1] i965: compiler option msse2 and mstackrealign

2018-09-04 Thread Sergii Romantsov 
Seems in case of 32-bit library, usage of msse2 makes
some stack corruption or incorrect instructions.
Usage with mstackrealign fixes that case.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107779
Signed-off-by: Sergii Romantsov 
---
 src/mesa/drivers/dri/i965/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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)
 
-- 
2.7.4

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