On 07/11/2022 14:56, Richard Biener wrote:
On Mon, 7 Nov 2022, Andre Vieira (lists) wrote:
On 07/11/2022 11:05, Richard Biener wrote:
On Fri, 4 Nov 2022, Andre Vieira (lists) wrote:
Sorry for the delay, just been reminded I still had this patch outstanding
from last stage 1. Hopefully since it has been mostly reviewed it could go
in
for this stage 1?
I addressed the comments and gave the slp-part of vectorizable_call some
TLC
to make it work.
I also changed vect_get_slp_defs as I noticed that the call from
vectorizable_call was creating an auto_vec with 'nargs' that might be less
than the number of children in the slp_node
how so? Please fix that in the caller. It looks like it probably
shoud use vect_nargs instead?
Well that was my first intuition, but when I looked at it further the variant
it's calling:
void vect_get_slp_defs (vec_info *, slp_tree slp_node, vec<vec<tree> >
*vec_oprnds, unsigned n)
Is actually creating a vector of vectors of slp defs. So for each child of
slp_node it calls:
void vect_get_slp_defs (slp_tree slp_node, vec<tree> *vec_defs)
Which returns a vector of vectorized defs. So vect_nargs would be the right
size for the inner vec<tree> of vec_defs, but the outer should have the same
number of elements as the original slp_node has children.
No, the inner vector is the vector of vectors for each arg, the outer
vector should be the one for each argument. Hm, that was a confusing
sentence.
That said, the number of SLP children of a call node should eventually
be the number of arguments of the call (plus masks, etc.). So it
looks about correct besides the vec_nargs issue?
Yeah you are right, I misunderstood what the children were, so you have
a child node for each argument of the call. Though, since you iterate
over the 'scalar' arguments of the call I actually think 'nargs' was
correct to begin with, which would explain why this never went wrong...
So I think it is actually correct as is, I must have gotten confused by
some earlier investigation into how to deal with the scalar arguments...
Sorry for the noise, I'll undo these changes to the patch.