diff mbox series

[v2,1/2] Allow vec_duplicate_optab to fail

Message ID 20210605151844.470344-2-hjl.tools@gmail.com
State New
Headers show
Series Allow vec_duplicate_optab to fail | expand

Commit Message

H.J. Lu June 5, 2021, 3:18 p.m. UTC 
Update vec_duplicate to allow to fail so that backend can only allow
broadcasting an integer constant to a vector when broadcast instruction
is available.

	* expr.c (store_constructor): Replace expand_insn with
	maybe_expand_insn for vec_duplicate_optab.
	* doc/md.texi: Update vec_duplicate.
---
 gcc/doc/md.texi |  2 --
 gcc/expr.c      | 10 ++++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Richard Sandiford June 7, 2021, 7:12 a.m. UTC | #1 
"H.J. Lu" <hjl.tools@gmail.com> writes:
> Update vec_duplicate to allow to fail so that backend can only allow
> broadcasting an integer constant to a vector when broadcast instruction
> is available.

I'm not sure why we need this to fail though.  Once the optab is defined
for target X, the optab should handle all duplicates for target X,
even if there are different strategies it can use.

AIUI the case you want to make conditional is the constant case.
I guess the first question is: why don't we simplify those CONSTRUCTORs
to VECTOR_CSTs in gimple?  I'm surprised we still see the constant case
as a constructor here.

If we can't rely on that happening, then would it work to change:

	/* Try using vec_duplicate_optab for uniform vectors.  */
	if (!TREE_SIDE_EFFECTS (exp)
	    && VECTOR_MODE_P (mode)
	    && eltmode == GET_MODE_INNER (mode)
	    && ((icode = optab_handler (vec_duplicate_optab, mode))
		!= CODE_FOR_nothing)
	    && (elt = uniform_vector_p (exp)))

to something like:

	/* Try using vec_duplicate_optab for uniform vectors.  */
	if (!TREE_SIDE_EFFECTS (exp)
	    && VECTOR_MODE_P (mode)
	    && eltmode == GET_MODE_INNER (mode)
	    && (elt = uniform_vector_p (exp)))
	  {
	    if (TREE_CODE (elt) == INTEGER_CST
		|| TREE_CODE (elt) == POLY_INT_CST
		|| TREE_CODE (elt) == REAL_CST
		|| TREE_CODE (elt) == FIXED_CST)
	      {
		rtx src = gen_const_vec_duplicate (mode, expand_normal (node));
		emit_move_insn (target, src);
		break;
	      }
	    …
	  }

Thanks,
Richard
H.J. Lu June 7, 2021, 2:18 p.m. UTC | #2 
On Mon, Jun 7, 2021 at 12:12 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> "H.J. Lu" <hjl.tools@gmail.com> writes:
> > Update vec_duplicate to allow to fail so that backend can only allow
> > broadcasting an integer constant to a vector when broadcast instruction
> > is available.
>
> I'm not sure why we need this to fail though.  Once the optab is defined
> for target X, the optab should handle all duplicates for target X,
> even if there are different strategies it can use.
>
> AIUI the case you want to make conditional is the constant case.
> I guess the first question is: why don't we simplify those CONSTRUCTORs
> to VECTOR_CSTs in gimple?  I'm surprised we still see the constant case
> as a constructor here.

The particular testcase for vec_duplicate is gcc.dg/pr100239.c.

> If we can't rely on that happening, then would it work to change:
>
>         /* Try using vec_duplicate_optab for uniform vectors.  */
>         if (!TREE_SIDE_EFFECTS (exp)
>             && VECTOR_MODE_P (mode)
>             && eltmode == GET_MODE_INNER (mode)
>             && ((icode = optab_handler (vec_duplicate_optab, mode))
>                 != CODE_FOR_nothing)
>             && (elt = uniform_vector_p (exp)))
>
> to something like:
>
>         /* Try using vec_duplicate_optab for uniform vectors.  */
>         if (!TREE_SIDE_EFFECTS (exp)
>             && VECTOR_MODE_P (mode)
>             && eltmode == GET_MODE_INNER (mode)
>             && (elt = uniform_vector_p (exp)))
>           {
>             if (TREE_CODE (elt) == INTEGER_CST
>                 || TREE_CODE (elt) == POLY_INT_CST
>                 || TREE_CODE (elt) == REAL_CST
>                 || TREE_CODE (elt) == FIXED_CST)
>               {
>                 rtx src = gen_const_vec_duplicate (mode, expand_normal (node));
>                 emit_move_insn (target, src);
>                 break;
>               }
>             …
>           }

I will give it a try.

Thanks.
Richard Biener June 7, 2021, 5:59 p.m. UTC | #3 
On Mon, Jun 7, 2021 at 4:19 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Jun 7, 2021 at 12:12 AM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
> >
> > "H.J. Lu" <hjl.tools@gmail.com> writes:
> > > Update vec_duplicate to allow to fail so that backend can only allow
> > > broadcasting an integer constant to a vector when broadcast instruction
> > > is available.
> >
> > I'm not sure why we need this to fail though.  Once the optab is defined
> > for target X, the optab should handle all duplicates for target X,
> > even if there are different strategies it can use.
> >
> > AIUI the case you want to make conditional is the constant case.
> > I guess the first question is: why don't we simplify those CONSTRUCTORs
> > to VECTOR_CSTs in gimple?  I'm surprised we still see the constant case
> > as a constructor here.
>
> The particular testcase for vec_duplicate is gcc.dg/pr100239.c.
>
> > If we can't rely on that happening, then would it work to change:
> >
> >         /* Try using vec_duplicate_optab for uniform vectors.  */
> >         if (!TREE_SIDE_EFFECTS (exp)
> >             && VECTOR_MODE_P (mode)
> >             && eltmode == GET_MODE_INNER (mode)
> >             && ((icode = optab_handler (vec_duplicate_optab, mode))
> >                 != CODE_FOR_nothing)
> >             && (elt = uniform_vector_p (exp)))
> >
> > to something like:
> >
> >         /* Try using vec_duplicate_optab for uniform vectors.  */
> >         if (!TREE_SIDE_EFFECTS (exp)
> >             && VECTOR_MODE_P (mode)
> >             && eltmode == GET_MODE_INNER (mode)
> >             && (elt = uniform_vector_p (exp)))
> >           {
> >             if (TREE_CODE (elt) == INTEGER_CST
> >                 || TREE_CODE (elt) == POLY_INT_CST
> >                 || TREE_CODE (elt) == REAL_CST
> >                 || TREE_CODE (elt) == FIXED_CST)
> >               {
> >                 rtx src = gen_const_vec_duplicate (mode, expand_normal (node));
> >                 emit_move_insn (target, src);
> >                 break;
> >               }
> >             …
> >           }
>
> I will give it a try.

I can confirm that veclower leaves us with an unfolded constant CTOR.
If you file a PR to remind me I'll fix that.

Richard.

> Thanks.
>
> --
> H.J.
Richard Biener June 7, 2021, 6:10 p.m. UTC | #4 
On Mon, Jun 7, 2021 at 7:59 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Mon, Jun 7, 2021 at 4:19 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Mon, Jun 7, 2021 at 12:12 AM Richard Sandiford
> > <richard.sandiford@arm.com> wrote:
> > >
> > > "H.J. Lu" <hjl.tools@gmail.com> writes:
> > > > Update vec_duplicate to allow to fail so that backend can only allow
> > > > broadcasting an integer constant to a vector when broadcast instruction
> > > > is available.
> > >
> > > I'm not sure why we need this to fail though.  Once the optab is defined
> > > for target X, the optab should handle all duplicates for target X,
> > > even if there are different strategies it can use.
> > >
> > > AIUI the case you want to make conditional is the constant case.
> > > I guess the first question is: why don't we simplify those CONSTRUCTORs
> > > to VECTOR_CSTs in gimple?  I'm surprised we still see the constant case
> > > as a constructor here.
> >
> > The particular testcase for vec_duplicate is gcc.dg/pr100239.c.
> >
> > > If we can't rely on that happening, then would it work to change:
> > >
> > >         /* Try using vec_duplicate_optab for uniform vectors.  */
> > >         if (!TREE_SIDE_EFFECTS (exp)
> > >             && VECTOR_MODE_P (mode)
> > >             && eltmode == GET_MODE_INNER (mode)
> > >             && ((icode = optab_handler (vec_duplicate_optab, mode))
> > >                 != CODE_FOR_nothing)
> > >             && (elt = uniform_vector_p (exp)))
> > >
> > > to something like:
> > >
> > >         /* Try using vec_duplicate_optab for uniform vectors.  */
> > >         if (!TREE_SIDE_EFFECTS (exp)
> > >             && VECTOR_MODE_P (mode)
> > >             && eltmode == GET_MODE_INNER (mode)
> > >             && (elt = uniform_vector_p (exp)))
> > >           {
> > >             if (TREE_CODE (elt) == INTEGER_CST
> > >                 || TREE_CODE (elt) == POLY_INT_CST
> > >                 || TREE_CODE (elt) == REAL_CST
> > >                 || TREE_CODE (elt) == FIXED_CST)
> > >               {
> > >                 rtx src = gen_const_vec_duplicate (mode, expand_normal (node));
> > >                 emit_move_insn (target, src);
> > >                 break;
> > >               }
> > >             …
> > >           }
> >
> > I will give it a try.
>
> I can confirm that veclower leaves us with an unfolded constant CTOR.
> If you file a PR to remind me I'll fix that.

The attached untested patch fixes this for the testcase.

Richard.

> Richard.
>
> > Thanks.
> >
> > --
> > H.J.
diff mbox series

Patch

diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 00caf3844cc..e66c41c4779 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -5077,8 +5077,6 @@  the mode appropriate for one element of @var{m}.
 This pattern only handles duplicates of non-constant inputs.  Constant
 vectors go through the @code{mov@var{m}} pattern instead.
 
-This pattern is not allowed to @code{FAIL}.
-
 @cindex @code{vec_series@var{m}} instruction pattern
 @item @samp{vec_series@var{m}}
 Initialize vector output operand 0 so that element @var{i} is equal to
diff --git a/gcc/expr.c b/gcc/expr.c
index e4660f0e90a..3107c32f259 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -7075,10 +7075,12 @@  store_constructor (tree exp, rtx target, int cleared, poly_int64 size,
 	    class expand_operand ops[2];
 	    create_output_operand (&ops[0], target, mode);
 	    create_input_operand (&ops[1], expand_normal (elt), eltmode);
-	    expand_insn (icode, 2, ops);
-	    if (!rtx_equal_p (target, ops[0].value))
-	      emit_move_insn (target, ops[0].value);
-	    break;
+	    if (maybe_expand_insn (icode, 2, ops))
+	      {
+		if (!rtx_equal_p (target, ops[0].value))
+		  emit_move_insn (target, ops[0].value);
+		break;
+	      }
 	  }
 
 	n_elts = TYPE_VECTOR_SUBPARTS (type);