Jason, Thank you for your review. You are correct, we only need to check has_discriminator for the statement that's on the same line. I updated the patch (attached).
Thanks, Eugene -----Original Message----- From: Jason Merrill <ja...@redhat.com> Sent: Thursday, August 18, 2022 6:55 PM To: Eugene Rozenfeld <eugene.rozenf...@microsoft.com>; gcc-patches@gcc.gnu.org Cc: Andi Kleen <a...@linux.intel.com>; Jan Hubicka <hubi...@ucw.cz> Subject: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support. On 8/3/22 17:25, Eugene Rozenfeld wrote: > One more ping for this patch > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc. > gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=0 > 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da81 > 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951 > %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I > k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2dT > DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0 > > CC Jason since this changes discriminators emitted in dwarf. > > Thanks, > > Eugene > > -----Original Message----- > From: Eugene Rozenfeld > Sent: Monday, June 27, 2022 12:45 PM > To: gcc-patches@gcc.gnu.org; Andi Kleen <a...@linux.intel.com>; Jan > Hubicka <hubi...@ucw.cz> > Subject: RE: [PING][PATCH] Add instruction level discriminator support. > > Another ping for > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=05%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da8185dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2dTDYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0 > . > > I got a review from Andi > (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596549.html&data=05%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da8185dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=se6x1LD0GQyFz%2B28gdVqsye3Aw8kPoMRhVQO1BSPg6I%3D&reserved=0) > but I also need a review from someone who can approve the changes. > > Thanks, > > Eugene > > -----Original Message----- > From: Eugene Rozenfeld > Sent: Friday, June 10, 2022 12:03 PM > To: gcc-patches@gcc.gnu.org; Andi Kleen <a...@linux.intel.com>; Jan > Hubicka <hubi...@ucw.cz> > Subject: [PING][PATCH] Add instruction level discriminator support. > > Hello, > > I'd like to ping this patch: > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc. > gnu.org%2Fpipermail%2Fgcc-patches%2F2022-June%2F596065.html&data=0 > 5%7C01%7Ceugene.rozenfeld%40microsoft.com%7C3e9ebe6dd5b14fe4471808da81 > 85dc68%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637964709325691951 > %7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I > k1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=K%2BMx6jelnED3n%2Be2dT > DYAPOqZZ8Zlsd2%2FyPJ0qib5%2FM%3D&reserved=0 > > Thanks, > > Eugene > > -----Original Message----- > From: Gcc-patches > <gcc-patches-bounces+erozen=microsoft....@gcc.gnu.org> On Behalf Of > Eugene Rozenfeld via Gcc-patches > Sent: Thursday, June 02, 2022 12:22 AM > To: gcc-patches@gcc.gnu.org; Andi Kleen <a...@linux.intel.com>; Jan > Hubicka <hubi...@ucw.cz> > Subject: [EXTERNAL] [PATCH] Add instruction level discriminator support. > > This is the first in a series of patches to enable discriminator support in > AutoFDO. > > This patch switches to tracking discriminators per statement/instruction > instead of per basic block. Tracking per basic block was problematic since > not all statements in a basic block needed a discriminator and, also, later > optimizations could move statements between basic blocks making correlation > during AutoFDO compilation unreliable. Tracking per statement also allows us > to assign different discriminators to multiple function calls in the same > basic block. A subsequent patch will add that support. > > The idea of this patch is based on commit > 4c311d95cf6d9519c3c20f641cc77af7df491fdf > by Dehao Chen in vendors/google/heads/gcc-4_8 but uses a slightly different > approach. In Dehao's work special (normally unused) location ids and side > tables were used to keep track of locations with discriminators. Things have > changed since then and I don't think we have unused location ids anymore. > Instead, I made discriminators a part of ad-hoc locations. > > The difference from Dehao's work also includes support for discriminator > reading/writing in lto streaming and in modules. > > Tested on x86_64-pc-linux-gnu. > @@ -1190,12 +1217,12 @@ assign_discriminators (void) > || (last && same_line_p (locus, &locus_e, > gimple_location (last)))) > { > - if (e->dest->discriminator != 0 && bb->discriminator == 0) > - bb->discriminator > - = next_discriminator_for_locus (locus_e.line); > + if (((first && has_discriminator (gimple_location (first))) > + || (last && has_discriminator (gimple_location (last)))) I think you want to check has_discriminator only for the one of first or last that we find to have the same line as locus above? Incidentally, I wonder why we ignore column number here, but that's not an issue for this patch. Jason
0001-Add-instruction-level-discriminator-support.patch
Description: 0001-Add-instruction-level-discriminator-support.patch
