From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 2E6FC3858C74 for ; Thu, 8 Sep 2022 17:25:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2E6FC3858C74 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1662657954; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=lwJmZ5A9RFX3OGgMiDz7l9CYMA5e9yWO6OgEGitPHEY=; b=eVwP6RGmxDJcmltPDMj/51MdSVnSWZn05GCoIY2hTK8RKVDmAtYPKOy7JV8c7YMpno4Co5 gLO1NajYs/1dv0nfzvAkp+xBCc/40tNxd9qPuQCs7DWFC56vWpzWkcZ9MutNs75m+h2b/y Gjsm/QcmYAr5ORsJQh4lPbPxLydGVc4= Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-264-BdzkPiauPzutjsMD5JgCmg-1; Thu, 08 Sep 2022 13:25:53 -0400 X-MC-Unique: BdzkPiauPzutjsMD5JgCmg-1 Received: by mail-qt1-f200.google.com with SMTP id o22-20020ac85a56000000b0034481129ce6so15061552qta.19 for ; Thu, 08 Sep 2022 10:25:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date; bh=lwJmZ5A9RFX3OGgMiDz7l9CYMA5e9yWO6OgEGitPHEY=; b=NWdgYOxIwaLV5tNZuGMRIcNDIKagxQ0tYUZkwZpVEvF+Eq3Der41m/dK61l1eBHw7L GhdeHY6bbMY+baEjjNp/aeP/CdTYPGkBk0Ai5T/nQJubw9isZTSSVjSApPruB0Xc0SwI ByMKp3g3tLD2W8xiK0IeHnDymDhmn502DnKhGZGhyd2i3H/2kAy/+yVu7Xu/1tkuITWK ndMw0SaRruvbysBEZUXfVmQDRAsLbJCr3IaIH/CyQs9sB8VQkUMPbVVLZchS6JS5WK6k OOmjLL35MW//xfcvrthRDbsrM6PGXQzadf3l81PrH+LbpfLkTFwQXrUrGPNNEuQLL0LW xQOQ== X-Gm-Message-State: ACgBeo2m7YfQrBQ/fqqkatNkWwRnNJNWGOQEQAb6ewGNGQgoRnO6ym1y 8UydjFPeIIuE9KoCM6/ih8ZzIM4P17E7GuxXC60e/jojQYL4/7m9h2+BYBTRgjRICMX+GaZiV/j zwVdtitx35GMnm9kT6w== X-Received: by 2002:a05:622a:199c:b0:344:7645:9ba1 with SMTP id u28-20020a05622a199c00b0034476459ba1mr8872906qtc.629.1662657953395; Thu, 08 Sep 2022 10:25:53 -0700 (PDT) X-Google-Smtp-Source: AA6agR5RA6b7VDfCuuLxDhsl8sjpWAHehfBDtx++F1jw4jh+CVmen7wR24uC1SkrfQ25Z44DcL5FZA== X-Received: by 2002:a05:622a:199c:b0:344:7645:9ba1 with SMTP id u28-20020a05622a199c00b0034476459ba1mr8872889qtc.629.1662657953123; Thu, 08 Sep 2022 10:25:53 -0700 (PDT) Received: from [192.168.1.101] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id v38-20020a05622a18a600b0035a7070e909sm783624qtc.38.2022.09.08.10.25.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 08 Sep 2022 10:25:52 -0700 (PDT) Message-ID: <863206fe-f5d6-932c-9b92-811f37774427@redhat.com> Date: Thu, 8 Sep 2022 13:25:51 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0 Subject: Re: [EXTERNAL] Re: [PING][PATCH] Add instruction level discriminator support. To: Eugene Rozenfeld , "gcc-patches@gcc.gnu.org" Cc: Andi Kleen , Jan Hubicka References: <5d9b2795-8a75-e4ea-07b1-856e448064dc@redhat.com> From: Jason Merrill In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-8.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_LOTSOFHASH,KAM_SHORT,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 On Behalf Of Eugene Rozenfeld via Gcc-patches > Sent: Thursday, September 01, 2022 1:49 PM > To: Jason Merrill ; gcc-patches@gcc.gnu.org > Cc: Andi Kleen ; Jan Hubicka > 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 > Sent: Thursday, August 18, 2022 6:55 PM > To: Eugene Rozenfeld ; gcc-patches@gcc.gnu.org > Cc: Andi Kleen ; Jan Hubicka > 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 ; Jan >> Hubicka >> 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 ; Jan >> Hubicka >> 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 >> On Behalf Of >> Eugene Rozenfeld via Gcc-patches >> Sent: Thursday, June 02, 2022 12:22 AM >> To: gcc-patches@gcc.gnu.org; Andi Kleen ; Jan >> Hubicka >> 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