diff mbox

Fix rs6000 stve* patterns (PR target/41082)

Message ID 20101208210050.GH29412@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Dec. 8, 2010, 9 p.m. UTC 
Hi!

The where_2.f90 testcase is miscompiled on ppc64-darwin with -O3 -m64
-mtune=power4.  The problem is in the altivec_stve* patterns, which
is represented as a V4SI mode store to MEM, but with offset already
adjusted, so e.g. DSE can believe memory after the end of the V4SI mode
temp is killed by the store.

Fixed by not using stve*x at all in rs6000_expand_vector_extract
(the temp slot is uninitialized and nothing ever looks at the other bytes,
so there is no point in avoiding storing other bytes - stvx should work
fine) and adjusting the patterns for the vec_stve* intrinsics so that
it represents what the insn actually does - stores only one scalar instead
of the whole vector.

Michael has bootstrapped/regtested this patch on power7.  Ok for trunk?

2010-12-08  Jakub Jelinek  <jakub@redhat.com>

	PR target/41082
	* config/rs6000/rs6000.c (rs6000_expand_vector_extract): Use stvx
	instead of stve*x.
	(altivec_expand_stv_builtin): For op0 use mode of operand 1 instead
	of operand 0.
	* config/rs6000/altivec.md (VI_scalar): New mode attr.
	(altivec_stve<VI_char>x, *altivec_stvesfx): Use scalar instead of
	vector mode for operand 0, put operand 1 into UNSPEC.


	Jakub

Comments

David Edelsohn Dec. 9, 2010, 3:10 a.m. UTC | #1 
On Wed, Dec 8, 2010 at 4:00 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The where_2.f90 testcase is miscompiled on ppc64-darwin with -O3 -m64
> -mtune=power4.  The problem is in the altivec_stve* patterns, which
> is represented as a V4SI mode store to MEM, but with offset already
> adjusted, so e.g. DSE can believe memory after the end of the V4SI mode
> temp is killed by the store.
>
> Fixed by not using stve*x at all in rs6000_expand_vector_extract
> (the temp slot is uninitialized and nothing ever looks at the other bytes,
> so there is no point in avoiding storing other bytes - stvx should work
> fine) and adjusting the patterns for the vec_stve* intrinsics so that
> it represents what the insn actually does - stores only one scalar instead
> of the whole vector.
>
> Michael has bootstrapped/regtested this patch on power7.  Ok for trunk?
>
> 2010-12-08  Jakub Jelinek  <jakub@redhat.com>
>
>        PR target/41082
>        * config/rs6000/rs6000.c (rs6000_expand_vector_extract): Use stvx
>        instead of stve*x.
>        (altivec_expand_stv_builtin): For op0 use mode of operand 1 instead
>        of operand 0.
>        * config/rs6000/altivec.md (VI_scalar): New mode attr.
>        (altivec_stve<VI_char>x, *altivec_stvesfx): Use scalar instead of
>        vector mode for operand 0, put operand 1 into UNSPEC.

Okay.

Thanks, David
diff mbox

Patch

--- gcc/config/rs6000/rs6000.c.jj	2010-12-02 11:51:30.000000000 +0100
+++ gcc/config/rs6000/rs6000.c	2010-12-08 08:25:40.000000000 +0100
@@ -5436,7 +5436,7 @@  rs6000_expand_vector_extract (rtx target
 {
   enum machine_mode mode = GET_MODE (vec);
   enum machine_mode inner_mode = GET_MODE_INNER (mode);
-  rtx mem, x;
+  rtx mem;
 
   if (VECTOR_MEM_VSX_P (mode) && (mode == V2DFmode || mode == V2DImode))
     {
@@ -5449,17 +5449,11 @@  rs6000_expand_vector_extract (rtx target
   /* Allocate mode-sized buffer.  */
   mem = assign_stack_temp (mode, GET_MODE_SIZE (mode), 0);
 
+  emit_move_insn (mem, vec);
+
   /* Add offset to field within buffer matching vector element.  */
-  mem = adjust_address_nv (mem, mode, elt * GET_MODE_SIZE (inner_mode));
+  mem = adjust_address_nv (mem, inner_mode, elt * GET_MODE_SIZE (inner_mode));
 
-  /* Store single field into mode-sized buffer.  */
-  x = gen_rtx_UNSPEC (VOIDmode,
-		      gen_rtvec (1, const0_rtx), UNSPEC_STVE);
-  emit_insn (gen_rtx_PARALLEL (VOIDmode,
-			       gen_rtvec (2,
-					  gen_rtx_SET (VOIDmode,
-						       mem, vec),
-					  x)));
   emit_move_insn (target, adjust_address_nv (mem, inner_mode, 0));
 }
 
@@ -11114,6 +11108,7 @@  altivec_expand_stv_builtin (enum insn_co
   rtx op2 = expand_normal (arg2);
   rtx pat, addr;
   enum machine_mode tmode = insn_data[icode].operand[0].mode;
+  enum machine_mode smode = insn_data[icode].operand[1].mode;
   enum machine_mode mode1 = Pmode;
   enum machine_mode mode2 = Pmode;
 
@@ -11123,8 +11118,8 @@  altivec_expand_stv_builtin (enum insn_co
       || arg2 == error_mark_node)
     return const0_rtx;
 
-  if (! (*insn_data[icode].operand[1].predicate) (op0, tmode))
-    op0 = copy_to_mode_reg (tmode, op0);
+  if (! (*insn_data[icode].operand[1].predicate) (op0, smode))
+    op0 = copy_to_mode_reg (smode, op0);
 
   op2 = copy_to_mode_reg (mode2, op2);
 
--- gcc/config/rs6000/altivec.md.jj	2010-11-19 20:56:52.000000000 +0100
+++ gcc/config/rs6000/altivec.md	2010-12-08 08:19:21.000000000 +0100
@@ -169,6 +169,7 @@  (define_mode_iterator VM [V4SI V8HI V16Q
 (define_mode_iterator VM2 [V4SI V8HI V16QI V4SF V2DF V2DI])
 
 (define_mode_attr VI_char [(V4SI "w") (V8HI "h") (V16QI "b")])
+(define_mode_attr VI_scalar [(V4SI "SI") (V8HI "HI") (V16QI "QI")])
 
 ;; Vector move instructions.
 (define_insn "*altivec_mov<mode>"
@@ -1775,19 +1776,15 @@  (define_insn "altivec_stvxl"
   [(set_attr "type" "vecstore")])
 
 (define_insn "altivec_stve<VI_char>x"
-  [(parallel
-    [(set (match_operand:VI 0 "memory_operand" "=Z")
-	  (match_operand:VI 1 "register_operand" "v"))
-     (unspec [(const_int 0)] UNSPEC_STVE)])]
+  [(set (match_operand:<VI_scalar> 0 "memory_operand" "=Z")
+	(unspec:<VI_scalar> [(match_operand:VI 1 "register_operand" "v")] UNSPEC_STVE))]
   "TARGET_ALTIVEC"
   "stve<VI_char>x %1,%y0"
   [(set_attr "type" "vecstore")])
 
 (define_insn "*altivec_stvesfx"
-  [(parallel
-    [(set (match_operand:V4SF 0 "memory_operand" "=Z")
-	  (match_operand:V4SF 1 "register_operand" "v"))
-     (unspec [(const_int 0)] UNSPEC_STVE)])]
+  [(set (match_operand:SF 0 "memory_operand" "=Z")
+	(unspec:SF [(match_operand:V4SF 1 "register_operand" "v")] UNSPEC_STVE))]
   "TARGET_ALTIVEC"
   "stvewx %1,%y0"
   [(set_attr "type" "vecstore")])