Re: combine BIT_FIELD_REF and VEC_PERM_EXPR

[Marc Glisse <marc dot glisse at inria dot fr>]

On Mon, 3 Sep 2012, Richard Guenther wrote:

> Please do the early outs where you compute the arguments.  Thus, right 
> after getting op0 in this case or right after computing n for the n != 1 
> check.

Ok.

> I think you need to verify that the type of 'op' is actually the element type
> of op0.  The BIT_FIELD_REF can happily access elements two and three
> of { 1, 2, 3, 4 } as a long for example.

Indeed I missed that.

> See the BIT_FIELD_REF foldings in fold-const.c.

That's what I was looking at (picked the same variable names size, idx, n) 
but I forgot that test :-(

> > +  if (code == VEC_PERM_EXPR)
> > +    {
> > +      tree p, m, index, tem;
> > +      unsigned nelts;
> > +      m = gimple_assign_rhs3 (def_stmt);
> > +      if (TREE_CODE (m) != VECTOR_CST)
> > +       return false;
> > +      nelts = VECTOR_CST_NELTS (m);
> > +      idx = TREE_INT_CST_LOW (VECTOR_CST_ELT (m, idx));
> > +      idx %= 2 * nelts;
> > +      if (idx < nelts)
> > +       {
> > +         p = gimple_assign_rhs1 (def_stmt);
> > +       }
> > +      else
> > +       {
> > +         p = gimple_assign_rhs2 (def_stmt);
> > +         idx -= nelts;
> > +       }
> > +      index = build_int_cst (TREE_TYPE (TREE_TYPE (m)), idx * size);
> > +      tem = fold_build3 (BIT_FIELD_REF, TREE_TYPE (op), p, op1, index);
>
> This shouldn't simplify, so you can use build3 instead.

I think that it is possible for p to be a VECTOR_CST, if the shuffle 
involves one constant and one non-constant vectors, no?

Now that I look at this line, I wonder if I am missing some unshare_expr 
for p and/or op1.

> Please also add handling of code == CONSTRUCTOR.

The cases I tried were already handled by fre1. I can add code for 
constructor, but I'll need to look for a testcase first. Can that go to a 
different patch?

--
Marc Glisse