Re: combine BIT_FIELD_REF and VEC_PERM_EXPR

[Richard Guenther <richard dot guenther at gmail dot com>]

On Mon, Sep 3, 2012 at 3:39 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> 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?

Well, constant propagation should have handled it ...

If you use fold_build3 you need to check that the result is in expected form
(a is_gimple_invariant or an SSA_NAME).

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

If either is a CONSTRUCTOR and its def stmt is not removed and it survives
into tem then yes ...

>> 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?

Yes.

Thanks,
Richard.

> --
> Marc Glisse