On 9/1/22 18:22, Eugene Rozenfeld wrote:
Jason,
I made another small change in addressing your feedback (attached).
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, September 01, 2022 1:49 PM
To: Jason Merrill <ja...@redhat.com>; gcc-patches@gcc.gnu.org
Cc: Andi Kleen <a...@linux.intel.com>; Jan Hubicka <hubi...@ucw.cz>
Subject: RE: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator
support.
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%7Cf217ebc45428465857bd08da8c5b6fb2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637976621612503972%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=b0kTdzWRyiwdtcEFasyNlKv1vj%2FqwnipN3776C7xWcg%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%7Cf217ebc45428465857bd08da8c5b6fb2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637976621612503972%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=qxjBUCcGiKXtR4%2BOJq%2FFQN11C2M6BBurTguOBOjWJDw%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
This seems like excessive redundancy, especially with the slightly
different names with slightly different semantics:
+++ b/gcc/input.h
+#define LOCATION_DISCRIMINATOR(LOC) \
+ ((IS_ADHOC_LOC (LOC)) ? get_discriminator_from_adhoc_loc (line_table, (LOC))
\
+ : 0)
+extern int get_discriminator_from_locus (location_t);
+++ b/libcpp/include/line-map.h
+extern unsigned get_discriminator_from_loc (line_maps *set, location_t loc);
Maybe gcc/input.h could overload get_discriminator_from_loc to take a
single argument and forward to the libcpp version with 'line_table', and
then remove both the _locus version and the macro?
Jason
