Patchwork [4.8,i386] : Fix PR 57356, SSE2 instructions generated with '-mno-sse2'

login
register
mail settings
Submitter Uros Bizjak
Date May 21, 2013, 5:03 p.m.
Message ID <CAFULd4a_zWk8CFKKVjaWts3K2=8CDH7zXOfhZHfbL_BosOg7Rw@mail.gmail.com>
Download mbox | patch
Permalink /patch/245364/
State New
Headers show

Comments

Uros Bizjak - May 21, 2013, 5:03 p.m.
Hello!

This patch avoids movdqu/movdqa when SSE2 is disabled. Although
-mno-sse2 is bordering on ABI violation for 64bit targets, the patch
is simple enough to fix movti_internal_rex64 pattern.

2013-05-21  Uros Bizjak  <ubizjak@gmail.com>

    PR target/57356
    * config/i386/i386.md (*movti_internal_rex64): Emit movaps/movups
    for non-sse2 targets.

Tested on x86_64-pc-linux-gnu {,-m32}. OK for 4.8 branch?

(The patch is also needed for 4.7 branch, but not for mainline, where
this problem is fixed by movti_internal/movti_internal_rex64 merge).

Uros.
Jakub Jelinek - May 22, 2013, 7:55 a.m.
On Tue, May 21, 2013 at 07:03:54PM +0200, Uros Bizjak wrote:
> This patch avoids movdqu/movdqa when SSE2 is disabled. Although
> -mno-sse2 is bordering on ABI violation for 64bit targets, the patch
> is simple enough to fix movti_internal_rex64 pattern.

If the TImode attr variant isn't valid for !SSE2, I'd find it clearer
to do
(match_test "!TARGET_SSE2") (const_string "V4SF")
much earlier in the cond, before TARGET_AVX case that returns TImode
(sure, when TARGET_AVX is true, then !TARGET_SSE2 is false), and
not added using ior to an unrelated condition.

That said, you are the maintainer, thus your decision; I think this is
fine for 4.8 today (if committed before RC2 is rolled).

> 2013-05-21  Uros Bizjak  <ubizjak@gmail.com>
> 
>     PR target/57356
>     * config/i386/i386.md (*movti_internal_rex64): Emit movaps/movups
>     for non-sse2 targets.
> 
> Tested on x86_64-pc-linux-gnu {,-m32}. OK for 4.8 branch?
> 
> (The patch is also needed for 4.7 branch, but not for mainline, where
> this problem is fixed by movti_internal/movti_internal_rex64 merge).

> Index: config/i386/i386.md
> ===================================================================
> --- config/i386/i386.md	(revision 199017)
> +++ config/i386/i386.md	(working copy)
> @@ -1805,7 +1805,8 @@
>  		 (const_string "V4SF")
>  	       (match_test "TARGET_AVX")
>  		 (const_string "TI")
> -	       (match_test "optimize_function_for_size_p (cfun)")
> +	       (ior (not (match_test "TARGET_SSE2"))
> +		    (match_test "optimize_function_for_size_p (cfun)"))
>  		 (const_string "V4SF")
>  	       ]
>  	       (const_string "TI")))])


	Jakub
Uros Bizjak - May 22, 2013, 8:01 a.m.
On Wed, May 22, 2013 at 9:55 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, May 21, 2013 at 07:03:54PM +0200, Uros Bizjak wrote:
>> This patch avoids movdqu/movdqa when SSE2 is disabled. Although
>> -mno-sse2 is bordering on ABI violation for 64bit targets, the patch
>> is simple enough to fix movti_internal_rex64 pattern.
>
> If the TImode attr variant isn't valid for !SSE2, I'd find it clearer
> to do
> (match_test "!TARGET_SSE2") (const_string "V4SF")
> much earlier in the cond, before TARGET_AVX case that returns TImode
> (sure, when TARGET_AVX is true, then !TARGET_SSE2 is false), and
> not added using ior to an unrelated condition.

Yes, this is a good suggestion, I was trying to be consistent with the
movti_internal pattern.

I will change both in the way you suggested ASAP.

Thanks,
Uros.

Patch

Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md	(revision 199017)
+++ config/i386/i386.md	(working copy)
@@ -1805,7 +1805,8 @@ 
 		 (const_string "V4SF")
 	       (match_test "TARGET_AVX")
 		 (const_string "TI")
-	       (match_test "optimize_function_for_size_p (cfun)")
+	       (ior (not (match_test "TARGET_SSE2"))
+		    (match_test "optimize_function_for_size_p (cfun)"))
 		 (const_string "V4SF")
 	       ]
 	       (const_string "TI")))])