On Fri, Mar 8, 2019 at 7:03 PM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Fri, Mar 8, 2019 at 9:49 AM H.J. Lu <hjl.to...@gmail.com> wrote:
> >
> > On Thu, Mar 7, 2019 at 9:51 AM H.J. Lu <hjl.to...@gmail.com> wrote:
> > >
> > > On Wed, Mar 6, 2019 at 8:33 PM Richard Biener
> > > <richard.guent...@gmail.com> wrote:
> > > >
> > > > On Wed, Mar 6, 2019 at 8:46 AM H.J. Lu <hjl.to...@gmail.com> wrote:
> > > > >
> > > > > On Tue, Mar 5, 2019 at 1:46 AM H.J. Lu <hjl.to...@gmail.com> wrote:
> > > > > >
> > > > > > On Mon, Mar 04, 2019 at 12:55:04PM +0100, Richard Biener wrote:
> > > > > > > On Sun, Mar 3, 2019 at 10:13 PM H.J. Lu <hjl.to...@gmail.com>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Sun, Mar 03, 2019 at 06:40:09AM -0800, Andrew Pinski wrote:
> > > > > > > > > )
> > > > > > > > > ,On Sun, Mar 3, 2019 at 6:32 AM H.J. Lu <hjl.to...@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > For vector init constructor:
> > > > > > > > > >
> > > > > > > > > > ---
> > > > > > > > > > typedef float __v4sf __attribute__ ((__vector_size__ (16)));
> > > > > > > > > >
> > > > > > > > > > __v4sf
> > > > > > > > > > foo (__v4sf x, float f)
> > > > > > > > > > {
> > > > > > > > > > __v4sf y = { f, x[1], x[2], x[3] };
> > > > > > > > > > return y;
> > > > > > > > > > }
> > > > > > > > > > ---
> > > > > > > > > >
> > > > > > > > > > we can optimize vector init constructor with vector copy or
> > > > > > > > > > permute
> > > > > > > > > > followed by a single scalar insert:
> > > > > >
> > > > > > > and you want to advance to the _1 = BIT_INSERT_EXPR here. The
> > > > > > > easiest way
> > > > > > > is to emit a new stmt for _2 = copy ...; and do the set_rhs with
> > > > > > > the
> > > > > > > BIT_INSERT_EXPR.
> > > > > >
> > > > > > Thanks for BIT_INSERT_EXPR suggestion. I am testing this patch.
> > > > > >
> > > > > >
> > > > > > H.J.
> > > > > > ---
> > > > > > We can optimize vector constructor with vector copy or permute
> > > > > > followed
> > > > > > by a single scalar insert:
> > > > > >
> > > > > > __v4sf y;
> > > > > > __v4sf D.1930;
> > > > > > float _1;
> > > > > > float _2;
> > > > > > float _3;
> > > > > >
> > > > > > <bb 2> :
> > > > > > _1 = BIT_FIELD_REF <x_9(D), 32, 96>;
> > > > > > _2 = BIT_FIELD_REF <x_9(D), 32, 64>;
> > > > > > _3 = BIT_FIELD_REF <x_9(D), 32, 32>;
> > > > > > y_6 = {f_5(D), _3, _2, _1};
> > > > > > return y_6;
> > > > > >
> > > > > > with
> > > > > >
> > > > > > __v4sf y;
> > > > > > __v4sf D.1930;
> > > > > > float _1;
> > > > > > float _2;
> > > > > > float _3;
> > > > > > vector(4) float _8;
> > > > > >
> > > > > > <bb 2> :
> > > > > > _1 = BIT_FIELD_REF <x_9(D), 32, 96>;
> > > > > > _2 = BIT_FIELD_REF <x_9(D), 32, 64>;
> > > > > > _3 = BIT_FIELD_REF <x_9(D), 32, 32>;
> > > > > > _8 = x_9(D);
> > > > > > y_6 = BIT_INSERT_EXPR <x_9(D), f_5(D), 0 (32 bits)>;
> > > > > > return y_6;
> > > > > >
> > > > > > gcc/
> > > > > >
> > > > > > PR tree-optimization/88828
> > > > > > * tree-ssa-forwprop.c (simplify_vector_constructor):
> > > > > > Optimize
> > > > > > vector init constructor with vector copy or permute followed
> > > > > > by a single scalar insert.
> > > > > >
> > > > > > gcc/testsuite/
> > > > > >
> > > > > > PR tree-optimization/88828
> > > > > > * gcc.target/i386/pr88828-1a.c: New test.
> > > > > > * gcc.target/i386/pr88828-2b.c: Likewise.
> > > > > > * gcc.target/i386/pr88828-2.c: Likewise.
> > > > > > * gcc.target/i386/pr88828-3a.c: Likewise.
> > > > > > * gcc.target/i386/pr88828-3b.c: Likewise.
> > > > > > * gcc.target/i386/pr88828-3c.c: Likewise.
> > > > > > * gcc.target/i386/pr88828-3d.c: Likewise.
> > > > > > * gcc.target/i386/pr88828-4a.c: Likewise.
> > > > > > * gcc.target/i386/pr88828-4b.c: Likewise.
> > > > > > * gcc.target/i386/pr88828-5a.c: Likewise.
> > > > > > * gcc.target/i386/pr88828-5b.c: Likewise.
> > > > > > * gcc.target/i386/pr88828-6a.c: Likewise.
> > > > > > * gcc.target/i386/pr88828-6b.c: Likewise.
> > > > >
> > > > > Here is the updated patch with run-time tests.
> > > >
> > > > - if (TREE_CODE (elt->value) != SSA_NAME)
> > > > + if (TREE_CODE (ce->value) != SSA_NAME)
> > > > return false;
> > > >
> > > > hmm, so it doesn't allow { 0, v[1], v[2], v[3] }? I think the single
> > > > scalar value can be a constant as well.
> > >
> > > Fixed.
> > >
> > > > if (!def_stmt)
> > > > - return false;
> > > > + {
> > > > + if (gimple_nop_p (SSA_NAME_DEF_STMT (ce->value)))
> > > >
> > > > if (SSA_NAME_IS_DEFAULT_DEF (ce->value))
> > > >
> > > > + {
> > > >
> > > > also you seem to disallow
> > > >
> > > > { i + 1, v[1], v[2], v[3] }
> > >
> > > Fixed by
> > >
> > > if (code != BIT_FIELD_REF)
> > > {
> > > /* Only allow one scalar insert. */
> > > if (nscalars != 0)
> > > return false;
> > >
> > > nscalars = 1;
> > > insert = true;
> > > scalar_idx = i;
> > > sel.quick_push (i);
> > > scalar_element = ce->value;
> > > continue;
> > > }
> > >
> > > > because get_prop_source_stmt will return the definition computing
> > > > i + 1 in this case and your code will be skipped?
> > > >
> > > > I think you can simplify the code by treating scalar_element != NULL
> > > > as nscalars == 1 and eliding nscalars.
> > >
> > > It works only if
> > >
> > > TYPE_VECTOR_SUBPARTS (type).to_constant () == (nscalars + nvectors)
> > >
> > > We need to check both nscalars and nvectors. Elide nscalar
> > > check doesn't help much here.
> > >
> > > > - if (conv_code == ERROR_MARK)
> > > > + if (conv_code == ERROR_MARK && !insert)
> > > > gimple_assign_set_rhs_with_ops (gsi, VEC_PERM_EXPR, orig[0],
> > > > orig[1], op2);
> > > > else
> > > > @@ -2148,10 +2198,25 @@ simplify_vector_constructor
> > > > (gimple_stmt_iterator *gsi)
> > > > VEC_PERM_EXPR, orig[0], orig[1],
> > > > op2);
> > > > orig[0] = gimple_assign_lhs (perm);
> > > > gsi_insert_before (gsi, perm, GSI_SAME_STMT);
> > > > - gimple_assign_set_rhs_with_ops (gsi, conv_code, orig[0],
> > > > + gimple_assign_set_rhs_with_ops (gsi,
> > > > + (conv_code != ERROR_MARK
> > > > + ? conv_code
> > > > + : NOP_EXPR),
> > > > + orig[0],
> > > > NULL_TREE, NULL_TREE);
> > > >
> > > > I believe you should elide the last stmt for conv_code == ERROR_MARK,
> > > > that is, why did you need to add the && !insert check in the guarding
> > > > condition
> > >
> > > When conv_code == ERROR_MARK, we still need
> > >
> > > gimple *perm
> > > = gimple_build_assign (make_ssa_name (TREE_TYPE (orig[0])),
> > > VEC_PERM_EXPR, orig[0], orig[1], op2);
> > > orig[0] = gimple_assign_lhs (perm);
> > > gsi_insert_before (gsi, perm, GSI_SAME_STMT);
> > > gimple_assign_set_rhs_with_ops (gsi, NOP_EXPR,
> > > orig[0],
> > > NULL_TREE, NULL_TREE);
> > >
> > > Otherwise, scalar insert won't work.
> > >
> > > > (this path should already do the correct thing?). Note that in all
> > > > cases it looks
> > > > that with conv_code != ERROR_MARK you may end up doing a float->int
> > > > or int->float conversion on a value it wasn't done on before which might
> > > > raise exceptions? That is, do we need to make sure we permute a
> > > > value we already do convert into the place we're going to insert to?
> > >
> > > This couldn't happen:
> > >
> > > if (type == TREE_TYPE (ref))
> > > {
> > > /* The RHS vector has the same type as LHS. */
> > > if (rhs_vector == NULL)
> > > rhs_vector = ref;
> > > /* Check if all RHS vector elements come fome the same
> > > vector. */
> > > if (rhs_vector == ref)
> > > nvectors++;
> > > }
> > > ...
> > > if (insert
> > > && (nvectors == 0
> > > || (TYPE_VECTOR_SUBPARTS (type).to_constant ()
> > > != (nscalars + nvectors))))
> > > return false;
>
> I see - that looks like a missed case then?
>
> { 1., (float)v[1], (float)v[2], (float)v[3] }
>
> with integer vector v?
True.
> I'll have a look at the full patch next week (it's GCC 10 material in any
> case).
>
Thanks.
--
H.J.
