On 11/19/20 1:01 PM, Segher Boessenkool wrote:
> On Thu, Nov 19, 2020 at 12:53:27PM -0700, Jeff Law wrote:
>> On 11/19/20 12:42 PM, Segher Boessenkool wrote:
>>> On Thu, Nov 19, 2020 at 12:13:34PM -0700, Jeff Law wrote:
>>>> On 8/31/20 9:33 PM, Jiufu Guo via Gcc-patches wrote:
>>>>> guojiufu <guoji...@linux.ibm.com> writes:
>>>>>> When unroll loops, if there are calls inside the loop, those calls
>>>>>> may raise negative impacts for unrolling. This patch adds a param
>>>>>> param_max_unrolled_calls, and checks if the number of calls inside
>>>>>> the loop bigger than this param, loop is prevent from unrolling.
>>>>>>
>>>>>> This patch is checking the _average_ number of calls which is the
>>>>>> summary of call numbers multiply the possibility of the call maybe
>>>>>> executed. The _average_ number could be a fraction, to keep the
>>>>>> precision, the param is the threshold number multiply 10000.
>>>>>>
>>>>>> Bootstrap and regtest pass on powerpc64le. Is this ok for trunk?
>>>>>>
>>>>>> gcc/ChangeLog
>>>>>> 2020-08-19 Jiufu Guo <guoji...@cn.ibm.com>
>>>>>>
>>>>>> * params.opt (param_max_unrolled_average_calls_x10000): New param.
>>>>>> * cfgloop.h (average_num_loop_calls): New declare.
>>>>>> * cfgloopanal.c (average_num_loop_calls): New function.
>>>>>> * loop-unroll.c (decide_unroll_constant_iteration,
>>>>>> decide_unroll_runtime_iterations,
>>>>>> decide_unroll_stupid): Check average_num_loop_calls and
>>>>>> param_max_unrolled_average_calls_x10000.
>>>> So what's the motivation behind adding a PARAM to control this
>>>> behavior? I'm not a big fan of exposing a lot of PARAMs for users to
>>>> tune behavior (though I've made the same lapse in judgment myself). In
>>>> my mind a PARAM is really more about controlling pathological behavior.
>>> But we (Power) need very different tuning than what others apparently
>>> need. It is similar to inlining, in that that also differs a lot
>>> between archs how aggressively to do that optimally.
>> But what I think that argues is that we've got a gap in the costing
>> model and/or how its being used. Throwing PARAMS at the problem isn't
>> really useful for the end user. The vast majority aren't going to use
>> them and of the ones that do, most are probably going to get it wrong.
> No, the vast majority of people will *not* (consciously) use them,
> because the target defaults will set things to useful values.
>
> The compiler could use saner "generic" defaults perhaps, but those will
> still not be satisfactory for anyone (except when they aren't generic in
> fact but instead tuned for one arch ;-) ) -- unrolling is just too
> important for performance.
Then fix the heuristics, don't add new PARAMS :-)
It didn't even occur to me until now that you may be pushing to have the
ppc backend have different values for the PARAMS. I would strongly
discourage that. It's been a huge headache in the s390 backend already.
>
>> In my mind fixing things so they work with no magic arguments is best.
>> PARAMS are the worst solution. A -f flag with no arguments is somewhere
>> in between. Others may clearly have different opinions here.
> There is no big difference between params and flags here, IMO -- it has
> to be a -f with a value as well, for good results.
Which is a signal that we have a deeper problem. -f with a value is no
different than a param.
>
> Since we have (almost) all such tunings in --param already, I'd say this
> one belongs there as well?
I'm not convinced at this point.
jeff
