Re: [PATCH][x86] Fix PR91814

[Richard Biener]

On Fri, 20 Sep 2019, Uros Bizjak wrote:

> On Thu, Sep 19, 2019 at 5:43 PM Uros Bizjak <ubiz...@gmail.com> wrote:
> >
> > On Thu, Sep 19, 2019 at 5:30 PM Richard Biener <rguent...@suse.de> wrote:
> > >
> > >
> > > Boostrapped and tested on x86_64-unknown-linux-gnu.
> > >
> > > OK?
> >
> > OK.
> 
> Hm, something is not working correctly here. For the testcase, I get:
> 
> main:
>         vmovq   %xmm0, %xmm0
>         vpxor   %xmm1, %xmm1, %xmm1
>         vpsubq  %xmm0, %xmm1, %xmm1
>         vpmaxsq %xmm1, %xmm0, %xmm0
>         vmovq   %xmm0, %rax
>         movabsq %rax, .LC0+11532131096
>         xorl    %eax, %eax
>         ret
> 
> The first insn uses uninitialized reg.
> 
> The _.stv pass misses initialization of r94 reg:
> 
> (note 2 3 7 2 NOTE_INSN_FUNCTION_BEG)
> (note 7 2 24 2 NOTE_INSN_DELETED)
> (insn 24 7 8 2 (set (subreg:V2DI (reg:DI 93) 0)
>         (vec_concat:V2DI (reg:DI 94)
>             (const_int 0 [0]))) "pr67271.c":11:17 -1
>      (nil))

Hmm, it emits the instruction in the wrong spot for the caller

      emit_insn_before (gen_rtx_SET (gen_rtx_SUBREG (vmode, tmp, 0),
                                     gen_gpr_to_xmm_move_src (vmode, 
*op)),
                        insn);

we can do the following which I am testing now.

Richard.

2019-09-20  Richard Biener  <rguent...@suse.de>

        PR target/91814
        * config/i386/i386-features.c (gen_gpr_to_xmm_move_src): Add
        before parameter to indicate where to emit a needed move to.
        (general_scalar_chain::convert_op): Use it.

Index: gcc/config/i386/i386-features.c
===================================================================
--- gcc/config/i386/i386-features.c     (revision 275989)
+++ gcc/config/i386/i386-features.c     (working copy)
@@ -666,18 +666,26 @@ scalar_chain::emit_conversion_insns (rtx
    zeroing the upper parts.  */
 
 static rtx
-gen_gpr_to_xmm_move_src (enum machine_mode vmode, rtx gpr)
+gen_gpr_to_xmm_move_src (enum machine_mode vmode, rtx gpr,
+                        rtx_insn *before = NULL)
 {
-  if (!nonimmediate_operand (gpr, GET_MODE_INNER (vmode)))
-    gpr = force_reg (GET_MODE_INNER (vmode), gpr);
+  enum machine_mode smode = GET_MODE_INNER (vmode);
+  if (!nonimmediate_operand (gpr, smode))
+    {
+      rtx tem = gen_reg_rtx (smode);
+      if (before)
+       emit_insn_before (gen_rtx_SET (tem, gpr), before);
+      else
+       emit_move_insn (tem, gpr);
+      gpr = tem;
+    }
   switch (GET_MODE_NUNITS (vmode))
     {
     case 1:
       /* We are not using this case currently.  */
       gcc_unreachable ();
     case 2:
-      return gen_rtx_VEC_CONCAT (vmode, gpr,
-                                CONST0_RTX (GET_MODE_INNER (vmode)));
+      return gen_rtx_VEC_CONCAT (vmode, gpr, CONST0_RTX (smode));
     default:
       return gen_rtx_VEC_MERGE (vmode, gen_rtx_VEC_DUPLICATE (vmode, gpr),
                                CONST0_RTX (vmode), GEN_INT (HOST_WIDE_INT_1U));
@@ -836,7 +844,8 @@ general_scalar_chain::convert_op (rtx *o
       rtx tmp = gen_reg_rtx (GET_MODE (*op));
 
       emit_insn_before (gen_rtx_SET (gen_rtx_SUBREG (vmode, tmp, 0),
-                                    gen_gpr_to_xmm_move_src (vmode, *op)),
+                                    gen_gpr_to_xmm_move_src (vmode, *op,
+                                                             insn)),
                        insn);
       *op = gen_rtx_SUBREG (vmode, tmp, 0);