diff mbox

[i386] : Fix ix86_spill_class condition

Message ID CAFULd4Z2iFncHUVPgkdG75J7v8Nm_J7ohLM=4k==yfU_qKXsLg@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak April 28, 2016, 12:21 a.m. UTC 
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.
diff mbox

Patch

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