On Fri, Jun 04, 2021 at 04:06:43PM +0200, Richard Biener wrote:
> On June 4, 2021 10:44:42 AM GMT+02:00, Jakub Jelinek <ja...@redhat.com> wrote:
> >Hi!
> >
> >The callers of fold_read_from_vector expect that the index they pass is
> >an index of an element in the vector and the function does that most of
> >the
> >time. But we allow CONSTRUCTORs with VECTOR_TYPE to have VECTOR_TYPE
> >elements and in that case every CONSTRUCTOR element represents not just
> >one
> >index (with the exception of V1 vectors), but multiple.
> >So returning zero vector if i >= CONSTRUCTOR_NELTS or returning some
> >CONSTRUCTOR_ELT's value might not be what the callers expect.
> >
> >Fixed by punting if the first element has vector type.
> >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> >In theory we could instead recurse (and assert that for CONSTRUCTORs of
> >vector elements we have always all elements specified like tree-cfg.c
> >verifies?) after adjusting the index appropriately.
>
> I think we do this in the corresponding BIT_FIELD_REF match.pd rule.
The fold_read_from_vector is indeed only called from the BIT_FIELD_REF
match.pd rule (3 times there), but I don't see there any number of elements
checking or this vector elt checking.
Do you want me to do this check in match.pd instead of
fold_read_from_vector?
> Note I don't think we allow CONSTRUCTOR (as in GENERIC) elements, so you'd
> only see SSA names with vector type here?
The match.pd rule is both GIMPLE and GENERIC, so I don't see why
it couldn't be there a VECTOR_TYPE CONSTRUCTOR with CONSTRUCTOR elements
(or e.g. one VAR_DECL element and one CONSTRUCTOR element).
In particular, on the testcase I see in *.original
W D.2844 = { 0, 0, 0, 0, 0, 0, 0, 0 };
return VIEW_CONVERT_EXPR<U>(BIT_FIELD_REF < VEC_PERM_EXPR < <<< Unknown tree:
compound_literal_expr
W D.2844 = { 0, 0, 0, 0, 0, 0, 0, 0 }; >>> , {v, { 0, 0, 0, 0 }} , { 0, 8,
2, 3, 4, 5, 6, 7 } > , 128, 0>);
In this case fold_read_from_vector is called on the {v, { 0, 0, 0, 0 }}
CONSTRUCTOR with 0 and so the recursion would be on VAR_DECL and would punt,
but if the permutation was e.g.
{ 0, 13, 2, 3, 4, 5, 6, 7 }
then it would be called with 5 as index and it could see that
it is in the second half (aka. the { 0, 0, 0, 0 } constructor) and
read the 5-4 element from there.
Even in GIMPLE, I see up to before veclower21
_1 = {v_3(D), { 0, 0, 0, 0 }};
_2 = VEC_PERM_EXPR <{ 0, 0, 0, 0, 0, 0, 0, 0 }, _1, { 0, 8, 2, 3, 4, 5, 6, 7
}>;
which seems like CONSTRUCTOR inside of CONSTRUCTOR.
But sure, it could be an SSA_NAME too and if we wanted to recurse it
would need to handle that case too and look at SSA_NAME_DEF_STMT.
Jakub
