On 02 May 10:50, Uros Bizjak wrote:
> On Fri, Apr 29, 2016 at 5:42 PM, Ilya Enkovich <enkovich....@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.enkov...@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.enkov...@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.enkov...@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.enkov...@intel.com>
* gcc.target/i386/pr70799-1.c: New test.
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;
+}
