Hi Honza,

Thanks for the review.

> On 13 Jan 2026, at 7:53 pm, Jan Hubicka <[email protected]> wrote:
>
> External email: Use caution opening links or attachments
>
>
>> I would like to again ping the patch posted in 
>> https://gcc.gnu.org/pipermail/gcc-patches/2025-November/700704.html
>>
>> Add hierarchical discriminator support for loop unrolling.
>> Assigns multiplicity and copyid discriminators to distinguish unrolled
>> iterations.
>>
>> gcc/ChangeLog:
>>
>>        * cfgloopmanip.cc (duplicate_loop_body_to_header_edge): Assign
>>        hierarchical discriminators for loop unrolling.
>>        * cfgloopmanip.h (DLTHE_RECORD_HIERARCHICAL_DISCRIMINATOR): New flag.
>>        * tree-ssa-loop-ivcanon.cc (try_unroll_loop_completely): Pass flag
>>        to enable hierarchical discriminator assignment.
>>        (try_peel_loop): Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>>        * gcc.dg/hierarchical-discriminator-unroll.c: New test.
>
> +  unsigned int loop_copyid_base = 0;
> +  if (current_ir_type () == IR_GIMPLE)
> +    {
> ..
> +    }
> +  else
> +    {
> +      /* For RTL, try to find an instruction with a valid location.
> +        Be defensive to avoid crashes on invalid/corrupted insns.  */
> +      rtx_insn *insn = BB_END (loop->header);
> +      while (insn && insn != BB_HEAD (loop->header))
> +       {
> +         /* Only check location if this is a valid insn.  */
> +         if (INSN_P (insn))
> +           {
> +             location_t loc = INSN_LOCATION (insn);
> +             if (loc != UNKNOWN_LOCATION)
> +               {
> +                 loop_loc = get_pure_location (loc);
> +                 break;
> +               }
> +           }
> +         insn = PREV_INSN (insn);
> +       }
> +    }
> +
> ...
> +         if (current_ir_type () == IR_GIMPLE)
> +           {
> ....
> +           }
> +         else
> +           {
> +             /* Only assign discriminators if we successfully allocated
> +                a copyid base.  */
> +             if (loop_copyid_base > 0)
>
> This will never be true.  But I think the copyid allocation can work
> both on RTL and Gimple.  You will need to support initializing the
> allocator at RTL level too if necessary, but both is just a siple walk
> of a function body.

Fixed this.


Is this OK?

Thanks,
Kugan


>
>
> Honza

Attachment: 0004-Autofdo-V5-Add-hierarchical-discriminator-for-loop-u.patch
Description: 0004-Autofdo-V5-Add-hierarchical-discriminator-for-loop-u.patch

Reply via email to