Re: [PATCH v8 2/2] Don't move cold code out of loop by checking bb count

[Xionghu Luo <luoxhu@linux.ibm.com>]

On 2021/12/20 15:29, Richard Biener wrote:
> On Wed, Dec 8, 2021 at 7:32 AM Xionghu Luo <luoxhu@linux.ibm.com> wrote:
>>
>>
>>
>> On 2021/12/7 20:17, Richard Biener wrote:
>>>>> +  class loop *coldest_loop = coldest_outermost_loop[loop->num];
>>>>> +  if (loop_depth (coldest_loop) < loop_depth (outermost_loop))
>>>>> +    {
>>>>> +      class loop *hotter_loop = hotter_than_inner_loop[loop->num];
>>>>> +      if (!hotter_loop
>>>>> +       || loop_depth (hotter_loop) < loop_depth (outermost_loop))
>>>>> +     return outermost_loop;
>>>>> +
>>>>> +      /*  hotter_loop is between OUTERMOST_LOOP and LOOP like:
>>>>> +     [loop tree root, ..., coldest_loop, ..., outermost_loop, ...,
>>>>> +     hotter_loop, second_coldest_loop, ..., loop]
>>>>> +     return second_coldest_loop to be the hoist target.  */
>>>>> +      class loop *aloop;
>>>>> +      for (aloop = hotter_loop->inner; aloop; aloop = aloop->next)
>>>>> +     if (flow_loop_nested_p (aloop, loop))
>>>> should be:
>>>>
>>>>         if (aloop == loop || flow_loop_nested_p (aloop, loop))
>>> OK with that fixed.
>>>
>>> Are necessary prerequesites committed to avoid regressions?
>>> I guess we need to keep a watchful eye and eventually revert
>>> (or gate with a --param disabled by default) the new behavior if
>>> severe regressions are discovered.
>>>
>>> Thanks and sorry for the repeated delays.
>>> Richard.
>>>
>>
>> Thanks for your review, I learned quite a lot and gained very useful
>> comments & help through the period :)  There are still 3 patches required
>> to avoid regression or so, I've reorganized them and sent it out.
>>
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586371.html
>>
>>
>> In addition, cooked the patch to add option for disable/enable it.
>> Is it OK to merge it to current patch?
> 
> Hmm, let's go without this flag for now, we can add something like this
> when we see a testcase where that makes sense (and where profile info
> is not wrecked by other bugs).
> 
> Adding a --param would be a no brainer, but for new user-facing options
> we should be more careful.
> 

Thanks.  Committed to r12-6087.


-- 
Thanks,
Xionghu