diff mbox

Vectorizing abs(char/short/int) on x86.

Message ID CAFULd4aq5FKYTCa+7vCepEkxv+g9t3b-XpfL3Q+Zw5F_bN1q9w@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak Oct. 31, 2013, 6:43 p.m. UTC 
On Wed, Oct 30, 2013 at 9:02 PM, Cong Hou <congh@google.com> wrote:
> I have run check_GNU_style.sh on my patch.
>
> The patch is submitted. Thank you for your comments and help on this patch!

I have committed a couple of fixes/improvements to your expander in
i386.c. There is no need to check for the result of
expand_simple_binop. Also, there is no guarantee that
expand_simple_binop will expand to the target. It can return different
RTX. Also, unhandled modes are now marked with gcc_unreachable.

2013-10-31  Uros Bizjak  <ubizjak@gmail.com>

    * config/i386/i386.c (ix86_expand_sse2_abs): Rename function arguments.
    Use gcc_unreachable for unhandled modes.  Do not check results of
    expand_simple_binop.  If not expanded to target, move the result.

Tested on x86_64-pc-linux-gnu and committed.

Uros.

Comments

Cong Hou Oct. 31, 2013, 8:20 p.m. UTC | #1 
This update makes it more safe. You showed me how to write better
expand code. Thank you for the improvement!



thanks,
Cong


On Thu, Oct 31, 2013 at 11:43 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Oct 30, 2013 at 9:02 PM, Cong Hou <congh@google.com> wrote:
>> I have run check_GNU_style.sh on my patch.
>>
>> The patch is submitted. Thank you for your comments and help on this patch!
>
> I have committed a couple of fixes/improvements to your expander in
> i386.c. There is no need to check for the result of
> expand_simple_binop. Also, there is no guarantee that
> expand_simple_binop will expand to the target. It can return different
> RTX. Also, unhandled modes are now marked with gcc_unreachable.
>
> 2013-10-31  Uros Bizjak  <ubizjak@gmail.com>
>
>     * config/i386/i386.c (ix86_expand_sse2_abs): Rename function arguments.
>     Use gcc_unreachable for unhandled modes.  Do not check results of
>     expand_simple_binop.  If not expanded to target, move the result.
>
> Tested on x86_64-pc-linux-gnu and committed.
>
> Uros.
diff mbox

Patch

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 204264)
+++ config/i386/i386.c	(working copy)
@@ -42020,36 +42020,36 @@ 
   return false;
 }
 
+/* Calculate integer abs() using only SSE2 instructions.  */
+
 void
-ix86_expand_sse2_abs (rtx op0, rtx op1)
+ix86_expand_sse2_abs (rtx target, rtx input)
 {
   enum machine_mode mode = GET_MODE (op0);
-  rtx tmp0, tmp1;
+  rtx tmp0, tmp1, x;
 
   switch (mode)
     {
       /* For 32-bit signed integer X, the best way to calculate the absolute
 	 value of X is (((signed) X >> (W-1)) ^ X) - ((signed) X >> (W-1)).  */
       case V4SImode:
-	tmp0 = expand_simple_binop (mode, ASHIFTRT, op1,
+	tmp0 = expand_simple_binop (mode, ASHIFTRT, input,
 				    GEN_INT (GET_MODE_BITSIZE
-						 (GET_MODE_INNER (mode)) - 1),
+					     (GET_MODE_INNER (mode)) - 1),
 				    NULL, 0, OPTAB_DIRECT);
-	if (tmp0)
-	  tmp1 = expand_simple_binop (mode, XOR, op1, tmp0,
-				      NULL, 0, OPTAB_DIRECT);
-	if (tmp0 && tmp1)
-	  expand_simple_binop (mode, MINUS, tmp1, tmp0,
-			       op0, 0, OPTAB_DIRECT);
+	tmp1 = expand_simple_binop (mode, XOR, tmp0, input,
+				    NULL, 0, OPTAB_DIRECT);
+	x = expand_simple_binop (mode, MINUS, tmp1, tmp0,
+				 output, 0, OPTAB_DIRECT);
 	break;
 
       /* For 16-bit signed integer X, the best way to calculate the absolute
 	 value of X is max (X, -X), as SSE2 provides the PMAXSW insn.  */
       case V8HImode:
 	tmp0 = expand_unop (mode, neg_optab, op1, NULL_RTX, 0);
-	if (tmp0)
-	  expand_simple_binop (mode, SMAX, op1, tmp0, op0, 0,
-			       OPTAB_DIRECT);
+
+	x = expand_simple_binop (mode, SMAX, tmp0, input,
+				 output, 0, OPTAB_DIRECT);
 	break;
 
       /* For 8-bit signed integer X, the best way to calculate the absolute
@@ -42057,14 +42057,17 @@ 
 	 as SSE2 provides the PMINUB insn.  */
       case V16QImode:
 	tmp0 = expand_unop (mode, neg_optab, op1, NULL_RTX, 0);
-	if (tmp0)
-	  expand_simple_binop (V16QImode, UMIN, op1, tmp0, op0, 0,
-			       OPTAB_DIRECT);
+
+	x = expand_simple_binop (V16QImode, UMIN, tmp0, input,
+				 output, 0, OPTAB_DIRECT);
 	break;
 
       default:
-	break;
+	gcc_unreachable ();
     }
+
+  if (x != output)
+    emit_move_insn (output, x);
 }
 
 /* Expand an insert into a vector register through pinsr insn.