Message ID | 20230721230851.1981434-1-lhyatt@gmail.com |
---|---|
Headers |
Return-Path: <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9010:0:b0:3e4:2afc:c1 with SMTP id l16csp514120vqg; Fri, 21 Jul 2023 16:11:17 -0700 (PDT) X-Google-Smtp-Source: APBJJlHaLWkmuW5JLLCeN34ZZUnSjDULx7Q1N4P3HC4p42JI5rXDqPDXvwBBghX2o5UP8CI6xLkL X-Received: by 2002:aa7:da85:0:b0:51d:dbf1:c825 with SMTP id q5-20020aa7da85000000b0051ddbf1c825mr2807255eds.1.1689981077556; Fri, 21 Jul 2023 16:11:17 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689981077; cv=none; d=google.com; s=arc-20160816; b=KlBDRYVWtw+k7tXh0KQ/XBMQjMATAo1ybC0zjHv5cgwsZDvSOO2G1aCv3S063fTuxO fGeVibu9BWYuXwjMO8zJLXPUtzDNNVXjtRNLgXVDtF49m6rMpgYRqcSe/jpvUGmdQPNC oJm4c7gwpoe6fOLrnUSulqCyn7UcpiS1jRg4JjfLJeH6rbbx82spr05za/oA1ldbQpdz xFkC2htTjaEPegIyUq4rw8jUQurlM148zVyFUrPBc1Pe2ys3TdnJzoQGB08Aj1ljL68z 6JX5FHS4z1433D/COEVgrb+xVRQSxMtrYyWhgxbB3RYgU9EejLT1Ma4hoqGIT/9V5HUV 60XQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:cc :to:dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=4drpnbbMTcTQi7oGF+5h0KzZLK2mTamLhG4NTC4xBd8=; fh=hLxXrzU+VDBolomQxjoi9c6yn4Oij2Jaf7BaYMHGh24=; b=TprGwzxBLY/T6sKlrRJUluWJkL6HxRWHG6ElaVy59ic7Gl7yIOYLu4wRWipvQfJ9zg psN1skpZpvS7JfNAaYEsK2W8KQLVYj31+Ratwn1gyw4vMyEx0q+1qZNf9lX4CSmapQi8 P9BehUR2PDE983FpmKrxch6GxwKsnF1PhMx5dZuaXbxVpvOFRblf9KG+tPYRMtY91sr3 AA3g0vonMrjlwpRP6l022PiBGqc06zb99JtzjEAVIOwPMFbGjQfHz0N2m+0BdE6wijJi 9QtrCYHpAxPMRiTfHFQr4UoILJp/aBjQE6/JZbNDp7OhOi839henN8KtHJQaUlhLD2UZ ATpA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=SEkimAe4; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id ca19-20020aa7cd73000000b0051df3b287b3si3032550edb.49.2023.07.21.16.11.17 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Jul 2023 16:11:17 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=SEkimAe4; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DACD2388216B for <ouuuleilei@gmail.com>; Fri, 21 Jul 2023 23:09:46 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DACD2388216B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1689980986; bh=4drpnbbMTcTQi7oGF+5h0KzZLK2mTamLhG4NTC4xBd8=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=SEkimAe45vtRCy47Zgb3UkANWDcChlnRUtW5egTFdDb/Q7ZjAw36txZFpgh+YE8uk kAccKzo3yNCAIAhJD64jOLHFUeadSpZbjt7MiHKCDSs4E4r5WQAYQdWR6lXQ91sIE8 ZX1J5Y7UFteBva+BOq1INn6PuydDBUoVpncWW/gA= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-qt1-x830.google.com (mail-qt1-x830.google.com [IPv6:2607:f8b0:4864:20::830]) by sourceware.org (Postfix) with ESMTPS id 3969A3855587 for <gcc-patches@gcc.gnu.org>; Fri, 21 Jul 2023 23:08:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3969A3855587 Received: by mail-qt1-x830.google.com with SMTP id d75a77b69052e-403c653d934so19528721cf.2 for <gcc-patches@gcc.gnu.org>; Fri, 21 Jul 2023 16:08:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689980938; x=1690585738; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=4drpnbbMTcTQi7oGF+5h0KzZLK2mTamLhG4NTC4xBd8=; b=fJ9s6sZ4Bj65b/7IDFXS1TpSPJag5psZyCj+ABgc8Qk3xwXp7E/XPmagFV3uCv3KiX zH4kePOcuLQ5qWXZKEpVKf/bxRbDbX40xlNiVRPdZ8mmysabZLDT7nQnQH0R8tsz64v7 qBZfgZiUCW/Uw3ww9qlIPLpeuzMawdpqLxRUDQelxlB56UH3O84q3Rx9qa/lyK+GR4dq /YFrOYwfTLXJH63T9h1z6yEJRxJn6or4tt0LNUBXFp84uyfctQf4wZoq6Op+qdliukrm 5moA9qDqCmTmDrJa/K64dEWPZlawQ8e9cGB50f4xGhcHeINSadGSVmIXiC3M0zmvzF3A Mt8A== X-Gm-Message-State: ABy/qLZUho9NtwHrn4qqNHhGLblx8Z+i6TAmZE6ZCviXexe9S4pGP/NB ESKFc6zU5eTXPO3zddhHbJrlJf1nR+k= X-Received: by 2002:a05:622a:11d0:b0:403:e3ce:565b with SMTP id n16-20020a05622a11d000b00403e3ce565bmr2101654qtk.4.1689980937696; Fri, 21 Jul 2023 16:08:57 -0700 (PDT) Received: from localhost.localdomain (96-67-140-173-static.hfc.comcastbusiness.net. [96.67.140.173]) by smtp.gmail.com with ESMTPSA id r26-20020ac8521a000000b00402ed9adfa1sm1586754qtn.87.2023.07.21.16.08.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Jul 2023 16:08:57 -0700 (PDT) To: gcc-patches@gcc.gnu.org Cc: David Malcolm <dmalcolm@redhat.com>, Lewis Hyatt <lhyatt@gmail.com> Subject: [PATCH v3 0/4] diagnostics: libcpp: Overhaul locations for _Pragma tokens Date: Fri, 21 Jul 2023 19:08:47 -0400 Message-Id: <20230721230851.1981434-1-lhyatt@gmail.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3032.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Lewis Hyatt via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Lewis Hyatt <lhyatt@gmail.com> Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1772073598763845635 X-GMAIL-MSGID: 1772073598763845635 |
Series |
diagnostics: libcpp: Overhaul locations for _Pragma tokens
|
|
Message
Lewis Hyatt
July 21, 2023, 11:08 p.m. UTC
Hello- This is an update to the v2 patch series last sent in January: https://gcc.gnu.org/pipermail/gcc-patches/2023-January/609473.html While I did not receive any feedback on the v2 patches yet, they did need some rebasing on top of other recent commits to input.cc, so I thought it would be helpful to send them again now. The patches have not otherwise changed from v2, and the above-linked message explains how all the patches fit in with the original v1 series sent last November. Dave, I would appreciate it very much if you could please let me know what you think of this approach? I feel like the diagnostics we currently output for _Pragmas are worth improving. As a reminder, say for this example: ===== #define S "GCC diagnostic ignored \"oops" _Pragma(S) ===== We currently output: ===== file.cpp:2:24: warning: missing terminating " character 2 | _Pragma(S) | ^ ===== While after these patches, we would output: ====== <generated>:1:24: warning: missing terminating " character 1 | GCC diagnostic ignored "oops | ^ file.cpp:2:1: note: in <_Pragma directive> 2 | _Pragma(S) | ^~~~~~~ ====== Thanks! -Lewis
Comments
On Fri, 2023-07-21 at 19:08 -0400, Lewis Hyatt wrote: > Hello- > > This is an update to the v2 patch series last sent in January: > https://gcc.gnu.org/pipermail/gcc-patches/2023-January/609473.html > > While I did not receive any feedback on the v2 patches yet, they did > need some > rebasing on top of other recent commits to input.cc, so I thought it > would be > helpful to send them again now. The patches have not otherwise > changed from > v2, and the above-linked message explains how all the patches fit in > with the > original v1 series sent last November. > > Dave, I would appreciate it very much if you could please let me know > what you > think of this approach? I feel like the diagnostics we currently > output for _Pragmas are worth improving. As a reminder, say for this > example: > > ===== > #define S "GCC diagnostic ignored \"oops" > _Pragma(S) > ===== > > We currently output: > > ===== > file.cpp:2:24: warning: missing terminating " character > 2 | _Pragma(S) > | ^ > ===== > > While after these patches, we would output: > > ====== > <generated>:1:24: warning: missing terminating " character > 1 | GCC diagnostic ignored "oops > | ^ > file.cpp:2:1: note: in <_Pragma directive> > 2 | _Pragma(S) > | ^~~~~~~ > ====== > > Thanks! Hi Lewis; sorry for not responding to the v2 patches. I've started looking at the v3 patches in detail, but I have some high- level questions about memory usage: Am I right in thinking that the effect of this patch is that for every _Pragma in the source we will create a new line_map_ordinary, and a new buffer for the stringified content of that _Pragma, and that these allocations will persist for the rest of the compilation? (plus a little extra allocation within the "location_t" space from 0 to 0x7fffffff). It sounds like this will probably be a rounding error that won't be noticable in profiling, but did you attempt any such measurement of the memory usage before/after this patch on some real-world projects? Thanks Dave
On Fri, Jul 28, 2023 at 6:22 PM David Malcolm <dmalcolm@redhat.com> wrote: > > On Fri, 2023-07-21 at 19:08 -0400, Lewis Hyatt wrote: > > Hello- > > > > This is an update to the v2 patch series last sent in January: > > https://gcc.gnu.org/pipermail/gcc-patches/2023-January/609473.html > > > > While I did not receive any feedback on the v2 patches yet, they did > > need some > > rebasing on top of other recent commits to input.cc, so I thought it > > would be > > helpful to send them again now. The patches have not otherwise > > changed from > > v2, and the above-linked message explains how all the patches fit in > > with the > > original v1 series sent last November. > > > > Dave, I would appreciate it very much if you could please let me know > > what you > > think of this approach? I feel like the diagnostics we currently > > output for _Pragmas are worth improving. As a reminder, say for this > > example: > > > > ===== > > #define S "GCC diagnostic ignored \"oops" > > _Pragma(S) > > ===== > > > > We currently output: > > > > ===== > > file.cpp:2:24: warning: missing terminating " character > > 2 | _Pragma(S) > > | ^ > > ===== > > > > While after these patches, we would output: > > > > ====== > > <generated>:1:24: warning: missing terminating " character > > 1 | GCC diagnostic ignored "oops > > | ^ > > file.cpp:2:1: note: in <_Pragma directive> > > 2 | _Pragma(S) > > | ^~~~~~~ > > ====== > > > > Thanks! > > Hi Lewis; sorry for not responding to the v2 patches. > > I've started looking at the v3 patches in detail, but I have some high- > level questions about memory usage: > > Am I right in thinking that the effect of this patch is that for every > _Pragma in the source we will create a new line_map_ordinary, and a new > buffer for the stringified content of that _Pragma, and that these > allocations will persist for the rest of the compilation? (plus a > little extra allocation within the "location_t" space from 0 to > 0x7fffffff). > > It sounds like this will probably be a rounding error that won't be > noticable in profiling, but did you attempt any such measurement of the > memory usage before/after this patch on some real-world projects? > > Thanks > Dave > Thanks for looking at the patches, I appreciate it whenever you have time to get to them. This is a fair point about the memory usage, basically it means that each instance of a _Pragma has comparable memory footprint to a macro definition. (In addition to the overheads you mentioned, it also creates a macro map to generate a virtual location for the tokens, so that it's able to output the "in expansion of _Pragma" note. That part can be disabled with -ftrack-macro-expansion=0 at least.) I had the sense that _Pragma isn't used often enough for that to be a problem, but agreed it is worth checking. (I really hope this memory usage isn't an issue since there are also numerous PRs complaining about 32-bit limitations in location tracking, that make it tempting to explore 64-bit line maps or some other option someday too.) I tried one thing now, wxWidgets uses a lot of diagnostic pragmas wrapped up inside macros that use _Pragma. (See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55578). The testsuite contains a file allheaders.cpp which includes the whole library, so I tried compiling this into a pch, which I believe measures the entire memory footprint including the ordinary and macro line maps and the _Pragma strings. The resulting PCH sizes were: 279000173 bytes before the changes 279491345 bytes after the changes So 0.1% bigger. Happy to check other projects too, do you have any standard gotos? Maybe firefox or something I take it. I see your other response on patch #1, I am thinking about that and will reply later. Thanks again! -Lewis
On Sat, 2023-07-29 at 10:27 -0400, Lewis Hyatt wrote: > On Fri, Jul 28, 2023 at 6:22 PM David Malcolm <dmalcolm@redhat.com> > wrote: > > > > On Fri, 2023-07-21 at 19:08 -0400, Lewis Hyatt wrote: > > > Hello- > > > > > > This is an update to the v2 patch series last sent in January: > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-January/609473.html > > > > > > While I did not receive any feedback on the v2 patches yet, they > > > did > > > need some > > > rebasing on top of other recent commits to input.cc, so I thought > > > it > > > would be > > > helpful to send them again now. The patches have not otherwise > > > changed from > > > v2, and the above-linked message explains how all the patches fit > > > in > > > with the > > > original v1 series sent last November. > > > > > > Dave, I would appreciate it very much if you could please let me > > > know > > > what you > > > think of this approach? I feel like the diagnostics we currently > > > output for _Pragmas are worth improving. As a reminder, say for > > > this > > > example: > > > > > > ===== > > > #define S "GCC diagnostic ignored \"oops" > > > _Pragma(S) > > > ===== > > > > > > We currently output: > > > > > > ===== > > > file.cpp:2:24: warning: missing terminating " character > > > 2 | _Pragma(S) > > > | ^ > > > ===== > > > > > > While after these patches, we would output: > > > > > > ====== > > > <generated>:1:24: warning: missing terminating " character > > > 1 | GCC diagnostic ignored "oops > > > | ^ > > > file.cpp:2:1: note: in <_Pragma directive> > > > 2 | _Pragma(S) > > > | ^~~~~~~ > > > ====== > > > > > > Thanks! > > > > Hi Lewis; sorry for not responding to the v2 patches. > > > > I've started looking at the v3 patches in detail, but I have some > > high- > > level questions about memory usage: > > > > Am I right in thinking that the effect of this patch is that for > > every > > _Pragma in the source we will create a new line_map_ordinary, and a > > new > > buffer for the stringified content of that _Pragma, and that these > > allocations will persist for the rest of the compilation? (plus a > > little extra allocation within the "location_t" space from 0 to > > 0x7fffffff). > > > > It sounds like this will probably be a rounding error that won't be > > noticable in profiling, but did you attempt any such measurement of > > the > > memory usage before/after this patch on some real-world projects? > > > > Thanks > > Dave > > > > Thanks for looking at the patches, I appreciate it whenever you have > time to get to them. > > This is a fair point about the memory usage, basically it means that > each instance of a _Pragma has comparable memory footprint to a macro > definition. (In addition to the overheads you mentioned, it also > creates a macro map to generate a virtual location for the tokens, so > that it's able to output the "in expansion of _Pragma" note. That > part > can be disabled with -ftrack-macro-expansion=0 at least.) > > I had the sense that _Pragma isn't used often enough for that to be a > problem, but agreed it is worth checking. (I really hope this memory > usage isn't an issue since there are also numerous PRs complaining > about 32-bit limitations in location tracking, that make it tempting > to explore 64-bit line maps or some other option someday too.) > > I tried one thing now, wxWidgets uses a lot of diagnostic pragmas > wrapped up inside macros that use _Pragma. (See > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55578). The testsuite > contains a file allheaders.cpp which includes the whole library, so I > tried compiling this into a pch, which I believe measures the entire > memory footprint including the ordinary and macro line maps and the > _Pragma strings. The resulting PCH sizes were: > > 279000173 bytes before the changes > 279491345 bytes after the changes > > So 0.1% bigger. Happy to check other projects too, do you have any > standard gotos? Maybe firefox or something I take it. Thanks for doing that test; I think that slight increase on a heavy user of _Pragma is acceptable. > > I see your other response on patch #1, I am thinking about that and > will reply later. Thanks again! Thanks. Hope that my patch #1 response makes sense and that I'm not missing something about the way this works. Dave