Re: [PATCH v4] vect/rs6000: Support vector with length cost modeling

[Richard Sandiford <richard.sandiford@arm.com>]

"Kewen.Lin" <linkw@linux.ibm.com> writes:
>>> +      bool niters_known_p = LOOP_VINFO_NITERS_KNOWN_P (loop_vinfo);
>>> +      bool need_iterate_p
>>> +	= (!LOOP_VINFO_EPILOGUE_P (loop_vinfo)
>>> +	   && !vect_known_niters_smaller_than_vf (loop_vinfo));
>>> +
>>> +      /* Init min/max, shift and minus cost relative to single
>>> +	 scalar_stmt. For now we only use length-based partial vectors on
>>> +	 Power, target specific cost tweaking may be needed for other
>>> +	 ports in future.  */
>>> +      unsigned int min_max_cost = 2;
>>> +      unsigned int shift_cost = 1, minus_cost = 1;
>> 
>> Please instead add a scalar_min_max to vect_cost_for_stmt, and use
>> scalar_stmt for shift and minus.  There shouldn't be any Power things
>> hard-coded into target-independent code.
>> 
>
> Agree!  It's not good to leave them there.  I thought to wait and see
> if other targets which support vector with length can reuse this, or
> move it to target specific codes then if not sharable.  But anyway
> it looks not good, let's fix it.
>
> I had some concerns on vect_cost_for_stmt way, since it seems to allow
> more computations similar to min/max to be added like this, in long
> term it probably leads to the situtation like: scalar_min_max,
> scalar_div_expr, scalar_mul_expr ... an enum (cost types) bloat, it
> seems not good to maintain.

I guess doing that doesn't seem so bad to me :-)  I think it's been
a recurring problem that the current classification isn't fine-grained
enough for some cases.

> I noticed that i386 port ix86_add_stmt_cost will check stmt_info->stmt,
> whether is assignment and the subcode of the expression, it provides the
> chance to check the statement more fine-grain, not just as normal
> scalar_stmt/vector_stmt.
>
> For the case here, we don't have the stmt_info, but we know the type
> of computation(expression), how about to extend the hook add_stmt_cost
> with one extra tree_code type argument, by default it can be some
> unmeaningful code, for some needs like here, we specify the tree_code
> as the code of computation, like {MIN,MAX}_EXPR, then target specific
> add_stmt_cost can check this tree_code and adjust the cost accordingly.

If we do that, I guess we should “promote” code_helper out of
gimple-match.h and use that instead, so that we can handle
internal and built-in functions too.

Would like to hear Richard's opinion on the best way forward here.

Thanks,
Richard