diff mbox

[i386,PR,target/70799,1/2] Support constants in STV pass (DImode)

Message ID 20160505120325.GA46462@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich May 10, 2016, 3:41 p.m. UTC 
On 02 May 10:50, Uros Bizjak wrote:
> On Fri, Apr 29, 2016 at 5:42 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> > Hi,
> >
> > As the first part of PR70799 fix I'd like to add constants support for
> > DI-STV pass (which is also related to PR70763).  This patch adds CONST_INT
> > support as insn operands and extends cost model accordingly.
> >
> > Bootstrapped and regtested on x86_64-unknown-linux-gnu{-m32}.  OK for trunk?
> >
> > Thanks,
> > Ilya
> > --
> > gcc/
> >
> > 2016-04-29  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> >         PR target/70799
> >         PR target/70763
> >         * config/i386/i386.c (dimode_scalar_to_vector_candidate_p): Allow
> >         integer constants.
> >         (dimode_scalar_chain::vector_const_cost): New.
> >         (dimode_scalar_chain::compute_convert_gain): Handle constants.
> >         (dimode_scalar_chain::convert_op): Likewise.
> >         (dimode_scalar_chain::convert_insn): Likewise.
> >
> > gcc/testsuite/
> >
> > 2016-04-29  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> >         * gcc.target/i386/pr70799-1.c: New test.
> >> @@ -3639,6 +3675,22 @@ dimode_scalar_chain::convert_op (rtx *op, rtx_insn *insn)
> >           }
> >        *op = gen_rtx_SUBREG (V2DImode, *op, 0);
> >      }
> > +  else if (CONST_INT_P (*op))
> > +    {
> > +      rtx cst = const0_rtx;
> > +      rtx tmp = gen_rtx_SUBREG (V2DImode, gen_reg_rtx (DImode), 0);
> > +
> > +      /* Prefer all ones vector in case of -1.  */
> > +      if (constm1_operand (*op, GET_MODE (*op)))
> > +       cst = *op;
> > +      cst = gen_rtx_CONST_VECTOR (V2DImode, gen_rtvec (2, *op, cst));
> 
> It took me a while to decipher the above functionality ;)
> 
> Why not just:
> 
>   else if (CONST_INT_P (*op))
>     {
>       rtx tmp = gen_rtx_SUBREG (V2DImode, gen_reg_rtx (DImode), 0);
>       rtx vec;
> 
>       /* Prefer all ones vector in case of -1.  */
>       if (constm1_operand (*op, GET_MODE (*op)))
>     vec = CONSTM1_RTX (V2DImode);
>       else
>     vec = gen_rtx_CONST_VECTOR (V2DImode,
>                     gen_rtvec (2, *op, const0_rtx));
> 
>       if (!standard_sse_constant_p (vec, V2DImode))
>     vec = validize_mem (force_const_mem (V2DImode, vec));
> 
>       emit_insn_before (gen_move_insn (tmp, vec), insn);
>       *op = tmp;
>     }

Agree.  This is more readable.

> 
> Comparing this part to timode_scalar_chain::convert_insn, there is a
> NONDEBUG_INSN_P check. Do you need one in the above code? Also, TImode
> pass handles REG_EQUAL and REG_EQUIV notes. Does dimode pass also need
> this functionality?

timode_scalar_chain conversion code is just simpler because of restricted
external dependencies for a chain and everything is processed in convert_insn.
For dimode_scalar_chain NONDEBUG_INSN_P is checked in convert_reg code.
Also I don't think I need anything to do with notes because I don't make
PUT_MODE for register.  Register mode and register value are unchanged and
therefore both debug insns and notes may be skipped.

> 
> Uros.

Here is an updated version.  Bootstrap and regression testing is in progress.
OK for trunk if testing pass?

Thanks,
Ilya
--
gcc/

2016-05-05  Ilya Enkovich  <ilya.enkovich@intel.com>

	* config/i386/i386.c (dimode_scalar_to_vector_candidate_p): Allow
	integer constants.
	(dimode_scalar_chain::vector_const_cost): New.
	(dimode_scalar_chain::compute_convert_gain): Handle constants.
	(dimode_scalar_chain::convert_op): Likewise.
	(dimode_scalar_chain::convert_insn): Likewise.

gcc/testsuite/

2016-05-05  Ilya Enkovich  <ilya.enkovich@intel.com>

	* gcc.target/i386/pr70799-1.c: New test.

Comments

Uros Bizjak May 10, 2016, 3:50 p.m. UTC | #1 
On Tue, May 10, 2016 at 5:41 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> On 02 May 10:50, Uros Bizjak wrote:
>> On Fri, Apr 29, 2016 at 5:42 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> > Hi,
>> >
>> > As the first part of PR70799 fix I'd like to add constants support for
>> > DI-STV pass (which is also related to PR70763).  This patch adds CONST_INT
>> > support as insn operands and extends cost model accordingly.
>> >
>> > Bootstrapped and regtested on x86_64-unknown-linux-gnu{-m32}.  OK for trunk?
>> >
>> > Thanks,
>> > Ilya
>> > --
>> > gcc/
>> >
>> > 2016-04-29  Ilya Enkovich  <ilya.enkovich@intel.com>
>> >
>> >         PR target/70799
>> >         PR target/70763
>> >         * config/i386/i386.c (dimode_scalar_to_vector_candidate_p): Allow
>> >         integer constants.
>> >         (dimode_scalar_chain::vector_const_cost): New.
>> >         (dimode_scalar_chain::compute_convert_gain): Handle constants.
>> >         (dimode_scalar_chain::convert_op): Likewise.
>> >         (dimode_scalar_chain::convert_insn): Likewise.
>> >
>> > gcc/testsuite/
>> >
>> > 2016-04-29  Ilya Enkovich  <ilya.enkovich@intel.com>
>> >
>> >         * gcc.target/i386/pr70799-1.c: New test.
>> >> @@ -3639,6 +3675,22 @@ dimode_scalar_chain::convert_op (rtx *op, rtx_insn *insn)
>> >           }
>> >        *op = gen_rtx_SUBREG (V2DImode, *op, 0);
>> >      }
>> > +  else if (CONST_INT_P (*op))
>> > +    {
>> > +      rtx cst = const0_rtx;
>> > +      rtx tmp = gen_rtx_SUBREG (V2DImode, gen_reg_rtx (DImode), 0);
>> > +
>> > +      /* Prefer all ones vector in case of -1.  */
>> > +      if (constm1_operand (*op, GET_MODE (*op)))
>> > +       cst = *op;
>> > +      cst = gen_rtx_CONST_VECTOR (V2DImode, gen_rtvec (2, *op, cst));
>>
>> It took me a while to decipher the above functionality ;)
>>
>> Why not just:
>>
>>   else if (CONST_INT_P (*op))
>>     {
>>       rtx tmp = gen_rtx_SUBREG (V2DImode, gen_reg_rtx (DImode), 0);
>>       rtx vec;
>>
>>       /* Prefer all ones vector in case of -1.  */
>>       if (constm1_operand (*op, GET_MODE (*op)))
>>     vec = CONSTM1_RTX (V2DImode);
>>       else
>>     vec = gen_rtx_CONST_VECTOR (V2DImode,
>>                     gen_rtvec (2, *op, const0_rtx));
>>
>>       if (!standard_sse_constant_p (vec, V2DImode))
>>     vec = validize_mem (force_const_mem (V2DImode, vec));
>>
>>       emit_insn_before (gen_move_insn (tmp, vec), insn);
>>       *op = tmp;
>>     }
>
> Agree.  This is more readable.
>
>>
>> Comparing this part to timode_scalar_chain::convert_insn, there is a
>> NONDEBUG_INSN_P check. Do you need one in the above code? Also, TImode
>> pass handles REG_EQUAL and REG_EQUIV notes. Does dimode pass also need
>> this functionality?
>
> timode_scalar_chain conversion code is just simpler because of restricted
> external dependencies for a chain and everything is processed in convert_insn.
> For dimode_scalar_chain NONDEBUG_INSN_P is checked in convert_reg code.
> Also I don't think I need anything to do with notes because I don't make
> PUT_MODE for register.  Register mode and register value are unchanged and
> therefore both debug insns and notes may be skipped.
>
>>
>> Uros.
>
> Here is an updated version.  Bootstrap and regression testing is in progress.
> OK for trunk if testing pass?
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2016-05-05  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         * config/i386/i386.c (dimode_scalar_to_vector_candidate_p): Allow
>         integer constants.
>         (dimode_scalar_chain::vector_const_cost): New.
>         (dimode_scalar_chain::compute_convert_gain): Handle constants.
>         (dimode_scalar_chain::convert_op): Likewise.
>         (dimode_scalar_chain::convert_insn): Likewise.
>
> gcc/testsuite/
>
> 2016-05-05  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         * gcc.target/i386/pr70799-1.c: New test.

OK for mainline.

Thanks,
Uros.
diff mbox

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 9680aaf..020eb29 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2789,7 +2789,8 @@  dimode_scalar_to_vector_candidate_p (rtx_insn *insn)
     return convertible_comparison_p (insn);
 
   /* We are interested in DImode promotion only.  */
-  if (GET_MODE (src) != DImode
+  if ((GET_MODE (src) != DImode
+       && !CONST_INT_P (src))
       || GET_MODE (dst) != DImode)
     return false;
 
@@ -2809,24 +2810,31 @@  dimode_scalar_to_vector_candidate_p (rtx_insn *insn)
       return true;
 
     case MEM:
+    case CONST_INT:
       return REG_P (dst);
 
     default:
       return false;
     }
 
-  if (!REG_P (XEXP (src, 0)) && !MEM_P (XEXP (src, 0))
+  if (!REG_P (XEXP (src, 0))
+      && !MEM_P (XEXP (src, 0))
+      && !CONST_INT_P (XEXP (src, 0))
       /* Check for andnot case.  */
       && (GET_CODE (src) != AND
 	  || GET_CODE (XEXP (src, 0)) != NOT
 	  || !REG_P (XEXP (XEXP (src, 0), 0))))
       return false;
 
-  if (!REG_P (XEXP (src, 1)) && !MEM_P (XEXP (src, 1)))
+  if (!REG_P (XEXP (src, 1))
+      && !MEM_P (XEXP (src, 1))
+      && !CONST_INT_P (XEXP (src, 1)))
       return false;
 
-  if (GET_MODE (XEXP (src, 0)) != DImode
-      || GET_MODE (XEXP (src, 1)) != DImode)
+  if ((GET_MODE (XEXP (src, 0)) != DImode
+       && !CONST_INT_P (XEXP (src, 0)))
+      || (GET_MODE (XEXP (src, 1)) != DImode
+	  && !CONST_INT_P (XEXP (src, 1))))
     return false;
 
   return true;
@@ -3120,6 +3128,7 @@  class dimode_scalar_chain : public scalar_chain
   void convert_reg (unsigned regno);
   void make_vector_copies (unsigned regno);
   void convert_registers ();
+  int vector_const_cost (rtx exp);
 };
 
 class timode_scalar_chain : public scalar_chain
@@ -3328,6 +3337,19 @@  scalar_chain::build (bitmap candidates, unsigned insn_uid)
   BITMAP_FREE (queue);
 }
 
+/* Return a cost of building a vector costant
+   instead of using a scalar one.  */
+
+int
+dimode_scalar_chain::vector_const_cost (rtx exp)
+{
+  gcc_assert (CONST_INT_P (exp));
+
+  if (standard_sse_constant_p (exp, V2DImode))
+    return COSTS_N_INSNS (1);
+  return ix86_cost->sse_load[1];
+}
+
 /* Compute a gain for chain conversion.  */
 
 int
@@ -3359,11 +3381,25 @@  dimode_scalar_chain::compute_convert_gain ()
 	       || GET_CODE (src) == IOR
 	       || GET_CODE (src) == XOR
 	       || GET_CODE (src) == AND)
-	gain += ix86_cost->add;
+	{
+	  gain += ix86_cost->add;
+	  if (CONST_INT_P (XEXP (src, 0)))
+	    gain -= vector_const_cost (XEXP (src, 0));
+	  if (CONST_INT_P (XEXP (src, 1)))
+	    gain -= vector_const_cost (XEXP (src, 1));
+	}
       else if (GET_CODE (src) == COMPARE)
 	{
 	  /* Assume comparison cost is the same.  */
 	}
+      else if (GET_CODE (src) == CONST_INT)
+	{
+	  if (REG_P (dst))
+	    gain += COSTS_N_INSNS (2);
+	  else if (MEM_P (dst))
+	    gain += 2 * ix86_cost->int_store[2] - ix86_cost->sse_store[1];
+	  gain -= vector_const_cost (src);
+	}
       else
 	gcc_unreachable ();
     }
@@ -3639,6 +3675,24 @@  dimode_scalar_chain::convert_op (rtx *op, rtx_insn *insn)
 	  }
       *op = gen_rtx_SUBREG (V2DImode, *op, 0);
     }
+  else if (CONST_INT_P (*op))
+    {
+      rtx vec_cst;
+      rtx tmp = gen_rtx_SUBREG (V2DImode, gen_reg_rtx (DImode), 0);
+
+      /* Prefer all ones vector in case of -1.  */
+      if (constm1_operand (*op, GET_MODE (*op)))
+	vec_cst = CONSTM1_RTX (V2DImode);
+      else
+	vec_cst = gen_rtx_CONST_VECTOR (V2DImode,
+					gen_rtvec (2, *op, const0_rtx));
+
+      if (!standard_sse_constant_p (vec_cst, V2DImode))
+	vec_cst = validize_mem (force_const_mem (V2DImode, vec_cst));
+
+      emit_insn_before (gen_move_insn (tmp, vec_cst), insn);
+      *op = tmp;
+    }
   else
     {
       gcc_assert (SUBREG_P (*op));
@@ -3711,6 +3765,10 @@  dimode_scalar_chain::convert_insn (rtx_insn *insn)
 			    UNSPEC_PTEST);
       break;
 
+    case CONST_INT:
+      convert_op (&src, insn);
+      break;
+
     default:
       gcc_unreachable ();
     }
diff --git a/gcc/testsuite/gcc.target/i386/pr70799-1.c b/gcc/testsuite/gcc.target/i386/pr70799-1.c
new file mode 100644
index 0000000..0abbfb9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr70799-1.c
@@ -0,0 +1,41 @@ 
+/* PR target/pr70799 */
+/* { dg-do compile { target { ia32 } } } */
+/* { dg-options "-O2 -march=slm" } */
+/* { dg-final { scan-assembler "pxor" } } */
+/* { dg-final { scan-assembler "pcmpeqd" } } */
+/* { dg-final { scan-assembler "movdqa\[ \\t\]+.LC0" } } */
+
+long long a, b, c;
+
+void test1 (void)
+{
+  long long t;
+  if (a)
+    t = 0LL;
+  else
+    t = b;
+  a = c & t;
+  b = c | t;
+}
+
+void test2 (void)
+{
+  long long t;
+  if (a)
+    t = -1LL;
+  else
+    t = b;
+  a = c & t;
+  b = c | t;
+}
+
+void test3 (void)
+{
+  long long t;
+  if (a)
+    t = 0xf0f0f0f0f0f0f0f0LL;
+  else
+    t = b;
+  a = c & t;
+  b = c | t;
+}