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.

Reply via email to