> Hi Honza,
>
> Thanks for the review.
>
> > On 13 Jan 2026, at 7:44 pm, Jan Hubicka <[email protected]> wrote:
> >
> > External email: Use caution opening links or attachments
> >
> >
> >> Hi Honza,
> >>
> >>> On 12 Dec 2025, at 9:04 am, Kugan Vivekanandarajah
> >>> <[email protected]> wrote
> >>> How about going with something like:
> >>> copyid_base = allocate_copyid_base (loc, base_discriminator);
> >>> This should have global context and allocate new copyid_base for each
> >>> call without any of the defines we have now.
> >>> This would still have issue for LTO where any copyid_base generated in
> >>> LTRANS will have conflict unless we generate this
> >>> in WPA and stream. What do you think?
> >>
> >> Here is the implementation (just the patch 1 or the infrastructure). Do
> >> you prefer this?
> >>
> >> Using this API, we would assign copy_id like:
> >> + gimple *nloop_last = last_nondebug_stmt (nloop->header);
> >> + location_t nloop_loc
> >> + = nloop_last ? gimple_location (nloop_last) :
> >> UNKNOWN_LOCATION;
> >> + if (nloop_loc != UNKNOWN_LOCATION)
> >> + {
> >> + unsigned int nloop_copyid = allocate_copyid_base (nloop_loc, 1);
> >> + assign_discriminators_to_loop (nloop, 0, nloop_copyid);
> >> + }
> >>
> >>
> >> I will post the series based on this if you prefer this.
> > This looks good. However I think we have problem with LTO streaming. If
> > some duplication happens at compile time and some at ltrans time, copyid
> > allocator will not see the compile time allocations.
> >
> > Fortunately I also do not see why copyid needs to be unique across
> > clones, since we have inline stacks. Is there one?
> That is true, This is mostly for within a function.
>
> >
> > I think the allocator needs to be per function only (stored in struct
> > function). When it it is initialized first time it should walk the
> > function body and look for existing maximal copyids for given location.
>
> Fixed this.
> >
> > Also the locations may come from inlined function (have their inline
> > stacks) so I think it should not be keyed by base_location, but by
> > base_location:inline stack.
> >
> > I think the API can stay same with this change, so I will look at the
> > remaining patches.
> > Honza
>
> Bootstrapped and regression tested on aarch64-linux-gnu with no new
> regressions.
>
> Is this OK?
The change is still missing the walk of a body after initialization
which is needed for LTO and also after inlning when existing duplicities
can be brought to the function body. But I think it can be done
incrementally, so the patch is OK.
Honza
>
> Thanks,
> Kugan
>
>
> >>
> >> Thanks,
> >> Kugan
>
>
