Patchwork V2DI zero constant in GPR (PR target/56948)

login
register
mail settings
Submitter David Edelsohn
Date April 13, 2013, 7:48 p.m.
Message ID <CAGWvnymjxjOqzdQONVotfYZ30L0uxYV0PnOT=pDwwGR+F-47Kw@mail.gmail.com>
Download mbox | patch
Permalink /patch/236390/
State New
Headers show

Comments

David Edelsohn - April 13, 2013, 7:48 p.m.
V2DI mode is allowed in GPRs and the pattern predicate allows easy
vector constants but the pattern in vsx.md does not provide an
alternative for that case, which can lead to an ICE where the insn
does not satisfy its constraints.  The following patch adds an
alternative for this case.

I also noticed that the VSX movti_64bit pattern does not handle
loading constants into a GPR.

And both the movti_64bit and movti_32bit patterns use j->wa instead of
O->wa.  The "j" constraint will work because it will accept any mode,
but I think that an "O" constraint is more accurate for a scalar mode
like TImode.

Because the  failure depends on the details of register allocation, I
do not have a short testcase.

Comments?

Thanks, David

    PR target/56948
    * config/rs6000/vsx.md (vsx_mov<mode>): Add j->r alternative.
    (vsx_movti_64bit): Change j->wa to O->wa.  Add n->r alternative.
    (vsx_movti_32bit): Change j->wa to O->wa.

"<VSr>,Z,<VSr>,wa,Z,wa,r,Y,r,j,j,j,W,v,wZ"))]
   "VECTOR_MEM_VSX_P (<MODE>mode)
    && (register_operand (operands[0], <MODE>mode)
        || register_operand (operands[1], <MODE>mode))"
@@ -238,23 +238,24 @@
     case 6:
     case 7:
     case 8:
+    case 11:
       return "#";

     case 9:
     case 10:
       return "xxlxor %x0,%x0,%x0";

-    case 11:
+    case 12:
       return output_vec_const_move (operands);

-    case 12:
+    case 13:
       gcc_assert (MEM_P (operands[0])
           && GET_CODE (XEXP (operands[0], 0)) != PRE_INC
           && GET_CODE (XEXP (operands[0], 0)) != PRE_DEC
           && GET_CODE (XEXP (operands[0], 0)) != PRE_MODIFY);
       return "stvx %1,%y0";

-    case 13:
+    case 14:
       gcc_assert (MEM_P (operands[0])
           && GET_CODE (XEXP (operands[0], 0)) != PRE_INC
           && GET_CODE (XEXP (operands[0], 0)) != PRE_DEC
@@ -265,14 +266,14 @@
       gcc_unreachable ();
     }
 }
-  [(set_attr "type"
"vecstore,vecload,vecsimple,vecstore,vecload,vecsimple,*,*,*,vecsimple,vecsimple,*,vecstore,vecload")])
+  [(set_attr "type"
"vecstore,vecload,vecsimple,vecstore,vecload,vecsimple,*,*,*,vecsimple,vecsimple,*,*,vecstore,vecload")])

 ;; Unlike other VSX moves, allow the GPRs even for reloading, since a normal
 ;; use of TImode is for unions.  However for plain data movement, slightly
 ;; favor the vector loads
 (define_insn "*vsx_movti_64bit"
-  [(set (match_operand:TI 0 "nonimmediate_operand" "=Z,wa,wa,wa,v,
v,wZ,?Y,?r,?r")
-    (match_operand:TI 1 "input_operand"        "wa, Z,wa, j,W,wZ, v,
r, Y, r"))]
+  [(set (match_operand:TI 0 "nonimmediate_operand" "=Z,wa,wa,wa,v,
v,wZ,?Y,?r,?r,?r")
+    (match_operand:TI 1 "input_operand"        "wa, Z,wa, O,W,wZ, v,
r, Y, r, n"))]
   "TARGET_POWERPC64 && VECTOR_MEM_VSX_P (TImode)
    && (register_operand (operands[0], TImode)
        || register_operand (operands[1], TImode))"
@@ -303,18 +304,19 @@
     case 7:
     case 8:
     case 9:
+    case 10:
       return "#";

     default:
       gcc_unreachable ();
     }
 }
-  [(set_attr "type"
"vecstore,vecload,vecsimple,vecsimple,vecsimple,vecstore,vecload,*,*,*")
-   (set_attr "length" "     4,      4,        4,       4,         8,
     4,      4,8,8,8")])
+  [(set_attr "type"
"vecstore,vecload,vecsimple,vecsimple,vecsimple,vecstore,vecload,*,*,*,*")
+   (set_attr "length" "     4,      4,        4,       4,         8,
     4,      4,8,8,8,8")])

 (define_insn "*vsx_movti_32bit"
   [(set (match_operand:TI 0 "nonimmediate_operand" "=Z,wa,wa,wa,v,
v,wZ,Q,Y,????r,????r,????r,r")
-    (match_operand:TI 1 "input_operand"        "wa, Z,wa, j,W,wZ,
v,r,r,    Q,    Y,    r,n"))]
+    (match_operand:TI 1 "input_operand"        "wa, Z,wa, O,W,wZ,
v,r,r,    Q,    Y,    r,n"))]
   "! TARGET_POWERPC64 && VECTOR_MEM_VSX_P (TImode)
    && (register_operand (operands[0], TImode)
        || register_operand (operands[1], TImode))"
Michael Meissner - April 15, 2013, 11:28 p.m.
On Sat, Apr 13, 2013 at 03:48:13PM -0400, David Edelsohn wrote:
> V2DI mode is allowed in GPRs and the pattern predicate allows easy
> vector constants but the pattern in vsx.md does not provide an
> alternative for that case, which can lead to an ICE where the insn
> does not satisfy its constraints.  The following patch adds an
> alternative for this case.
> 
> I also noticed that the VSX movti_64bit pattern does not handle
> loading constants into a GPR.
> 
> And both the movti_64bit and movti_32bit patterns use j->wa instead of
> O->wa.  The "j" constraint will work because it will accept any mode,
> but I think that an "O" constraint is more accurate for a scalar mode
> like TImode.
> 
> Because the  failure depends on the details of register allocation, I
> do not have a short testcase.
> 
> Comments?

This looks right.  Too bad we probably can't combine j/O constraints, due to
them being used in asm.

Patch

Index: vsx.md
===================================================================
--- vsx.md    (revision 197940)
+++ vsx.md    (working copy)
@@ -207,8 +207,8 @@ 

 ;; VSX moves
 (define_insn "*vsx_mov<mode>"
-  [(set (match_operand:VSX_M 0 "nonimmediate_operand"
"=Z,<VSr>,<VSr>,?Z,?wa,?wa,*Y,*r,*r,<VSr>,?wa,v,wZ,v")
-    (match_operand:VSX_M 1 "input_operand"
"<VSr>,Z,<VSr>,wa,Z,wa,r,Y,r,j,j,W,v,wZ"))]
+  [(set (match_operand:VSX_M 0 "nonimmediate_operand"
"=Z,<VSr>,<VSr>,?Z,?wa,?wa,*Y,*r,*r,<VSr>,?wa,*r,v,wZ,v")
+    (match_operand:VSX_M 1 "input_operand"