On Tue, 24 Jun 2025, Jan Hubicka wrote: > > On Sun, 22 Jun 2025, Jan Hubicka wrote: > > > > > Hi, > > > auto-fdo is currently confused by a fact that all inlined functions get > > > locators with 0 discriminator, so it is not bale to distinguish multiple > > > inlined calls from single line. > > > > > > Discriminator is lost by calling LOCATION_LOCUS before copying it from > > > former call statement. I believe this is only intended to drop block > > > annotations. > > > > > > Bootstrapped/regtested x86_64-linux, OK? > > > > > > gcc/ChangeLog: > > > > > > * tree-inline.cc (expand_call_inline): Preserve discriminator. > > > > > > diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc > > > index dee2dfc2620..fa2641780a5 100644 > > > --- a/gcc/tree-inline.cc > > > +++ b/gcc/tree-inline.cc > > > @@ -5014,6 +5014,9 @@ expand_call_inline (basic_block bb, gimple *stmt, > > > copy_body_data *id, > > > /* We do want to assign a not UNKNOWN_LOCATION > > > BLOCK_SOURCE_LOCATION > > > to make inlined_function_outer_scope_p return true on this > > > BLOCK. */ > > > location_t loc = LOCATION_LOCUS (gimple_location (stmt)); > > > + if (loc != UNKNOWN_LOCATION) > > > + loc = location_with_discriminator > > > + (loc, get_discriminator_from_loc (gimple_location (stmt))); > > > > So this doesn't preserve the discriminator on UNKNOWN_LOCATION? Don't > > you maybe want > > > > if (has_discriminator (gimple_location (stmt))) > > loc = location_with_discriminator (loc, get_disc.... > > > > ? Also ... > > > > > if (loc == UNKNOWN_LOCATION) > > > loc = LOCATION_LOCUS (DECL_SOURCE_LOCATION (fn)); > > > if (loc == UNKNOWN_LOCATION) > > > > ... these no longer trigger when there's a discriminator, but we do > > want a != UNKNOWN_LOCATION LOCATION_LOCUS as the comment says. So > > better apply the discriminator last? > That is why I checked for loc != UNKNOWN_LOCATION. I did not expect > UNKNOWN_LOCATION to have discriminators. What they are good for?
I have no idea, this was simply a defensive review where it's no longer obvious that inlined_function_outer_scope_p would still work in all cases. > I am re-testing the change as suggested. I will commit it if tesitng > succeds (I am in China with somewhat limited SSH access and I would like > the lnt testers to pick that change soon, so I know what benchmarks I > need to analyze for afdo regressions.) LGTM. > Thanks, > Honza > > gcc/ChangeLog: > > * tree-inline.cc (expand_call_inline): Copy discriminators. > > diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc > index dee2dfc2620..7e0ac698e5e 100644 > --- a/gcc/tree-inline.cc > +++ b/gcc/tree-inline.cc > @@ -5018,6 +5018,9 @@ expand_call_inline (basic_block bb, gimple *stmt, > copy_body_data *id, > loc = LOCATION_LOCUS (DECL_SOURCE_LOCATION (fn)); > if (loc == UNKNOWN_LOCATION) > loc = BUILTINS_LOCATION; > + if (has_discriminator (gimple_location (stmt))) > + loc = location_with_discriminator > + (loc, get_discriminator_from_loc (gimple_location (stmt))); > id->block = make_node (BLOCK); > BLOCK_ABSTRACT_ORIGIN (id->block) = DECL_ORIGIN (fn); > BLOCK_SOURCE_LOCATION (id->block) = loc; > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
