[PATCH, i386]: Fix ix86_spill_class condition

[PATCH, i386]: Fix ix86_spill_class condition

[Uros Bizjak]

Hello!

Based on recent discussion, the attached patch fixes ix86_spill_class
condition. The spills to SSE registers are now enabled for real on
SSE2 target, where inter-unit moves to/from vector registers are
enabled.

Since this is new functionality, the patch can cause some minor
runtime regressions (or unwanted regmove chains), so IMO the beginning
of stage1 is appropriate timing for these kind of changes.

TARGET_GENERAL_REGS_SSE_SPILL flag is enabled by default on all Intel
Core processors, so the change will be picked by SPEC testers and any
problems will soon be detected.

2016-04-27  Uros Bizjak  <ubizjak@gmail.com>

    * config/i386/i386.c (ix86_spill_class): Enable for TARGET_SSE2 when
    inter-unit moves to/from vector registers are enabled.  Do not disable
    for TARGET_MMX.

Patch was bootstrapped and regression tested on x86_64-linux-gnu
{,-m32}, configured with --with-arch=corei7.

Committed to mainline SVN.

Uros.

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c  (revision 235516)
+++ config/i386/i386.c  (working copy)
@@ -53560,9 +53560,12 @@
 static reg_class_t
 ix86_spill_class (reg_class_t rclass, machine_mode mode)
 {
-  if (TARGET_SSE && TARGET_GENERAL_REGS_SSE_SPILL && ! TARGET_MMX
+  if (TARGET_GENERAL_REGS_SSE_SPILL
+      && TARGET_SSE2
+      && TARGET_INTER_UNIT_MOVES_TO_VEC
+      && TARGET_INTER_UNIT_MOVES_FROM_VEC
       && (mode == SImode || (TARGET_64BIT && mode == DImode))
-      && rclass != NO_REGS && INTEGER_CLASS_P (rclass))
+      && INTEGER_CLASS_P (rclass))
     return ALL_SSE_REGS;
   return NO_REGS;
 }

Re: [PATCH, i386]: Fix ix86_spill_class condition

[Uros Bizjak]

[-- Attachment #1: Type: text/plain, Size: 3152 bytes --]

On Wed, Apr 27, 2016 at 8:05 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
> Based on recent discussion, the attached patch fixes ix86_spill_class
> condition. The spills to SSE registers are now enabled for real on
> SSE2 target, where inter-unit moves to/from vector registers are
> enabled.
>
> Since this is new functionality, the patch can cause some minor
> runtime regressions (or unwanted regmove chains), so IMO the beginning
> of stage1 is appropriate timing for these kind of changes.
>
> TARGET_GENERAL_REGS_SSE_SPILL flag is enabled by default on all Intel
> Core processors, so the change will be picked by SPEC testers and any
> problems will soon be detected.
>
> 2016-04-27  Uros Bizjak  <ubizjak@gmail.com>
>
>     * config/i386/i386.c (ix86_spill_class): Enable for TARGET_SSE2 when
>     inter-unit moves to/from vector registers are enabled.  Do not disable
>     for TARGET_MMX.
>
> Patch was bootstrapped and regression tested on x86_64-linux-gnu
> {,-m32}, configured with --with-arch=corei7.
>
> Committed to mainline SVN.

And, yes - the patch did trigger a bootstrap failure for march=corei7
in 32bit libjava multilib.

Now we have much more moves between SSE and general registers, so
there are some peephole2s that are not prepared for the fact that
SImode and DImode values can also live in SSE registers. One example:

(define_peephole2
  [(set (match_operand:SI 0 "memory_operand")
    (match_operand:SI 1 "register_operand"))
   (set (match_operand:SI 2 "register_operand") (match_dup 1))
   (parallel [(set (match_dup 2)
           (ashiftrt:SI (match_dup 2) (const_int 31)))
           (clobber (reg:CC FLAGS_REG))])
   (set (match_operand:SI 3 "memory_operand") (match_dup 2))]
  "REGNO (operands[1]) != REGNO (operands[2])
   && peep2_reg_dead_p (2, operands[1])
   && peep2_reg_dead_p (4, operands[2])
   && !reg_mentioned_p (operands[2], operands[3])"
  [(set (match_dup 0) (match_dup 1))
   (parallel [(set (match_dup 1) (ashiftrt:SI (match_dup 1) (const_int 31)))
          (clobber (reg:CC FLAGS_REG))])
   (set (match_dup 3) (match_dup 1))])

It isn't hard to see the problem when operand 2 is a SSE register, since we get:

(insn 284 283 285 2 (parallel [
            (set (reg:SI 21 xmm0 [orig:92 _11 ] [92])
                (ashiftrt:SI (reg:SI 21 xmm0 [orig:92 _11 ] [92])
                    (const_int 31 [0x1f])))
            (clobber (reg:CC 17 flags))
        ]) /home/uros/gcc-svn/trunk/libjava/classpath/java/util/zip/ZipFile.java:755
565 {ashrsi3_cvt}
     (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))

Fortunately, we get an ICE, so these peephole2s problems will be easy
to catch. The fix is simply to change "register_operand" predicate to
"general_reg_operand" predicate that allows - as we already figured
out - only general registers.

2016-04-28  Uros Bizjak  <ubizjak@gmail.com>

    * config/i386/i386.md (sign_extend to memory peephole2s): Use
    general_reg_operand instead of register_operand predicate.

Bootstrapped on x86_64-linux-gnu (with 32-bit multilib), regression
test in progress.

Committed to mainline to restore bootstrap on corei7 autotesters.

Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 1104 bytes --]

Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md	(revision 235535)
+++ config/i386/i386.md	(working copy)
@@ -3992,8 +3992,8 @@
 ;; being split with the above splitter.
 (define_peephole2
   [(set (match_operand:SI 0 "memory_operand")
-	(match_operand:SI 1 "register_operand"))
-   (set (match_operand:SI 2 "register_operand") (match_dup 1))
+	(match_operand:SI 1 "general_reg_operand"))
+   (set (match_operand:SI 2 "general_reg_operand") (match_dup 1))
    (parallel [(set (match_dup 2)
 		   (ashiftrt:SI (match_dup 2) (const_int 31)))
 	       (clobber (reg:CC FLAGS_REG))])
@@ -4009,8 +4009,8 @@
 
 (define_peephole2
   [(set (match_operand:SI 0 "memory_operand")
-	(match_operand:SI 1 "register_operand"))
-   (parallel [(set (match_operand:SI 2 "register_operand")
+	(match_operand:SI 1 "general_reg_operand"))
+   (parallel [(set (match_operand:SI 2 "general_reg_operand")
 		   (ashiftrt:SI (match_dup 1) (const_int 31)))
 	       (clobber (reg:CC FLAGS_REG))])
    (set (match_operand:SI 3 "memory_operand") (match_dup 2))]