From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com [IPv6:2a00:1450:4864:20::52c]) by sourceware.org (Postfix) with ESMTPS id 6AD903857C60 for ; Fri, 25 Jun 2021 07:46:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6AD903857C60 Received: by mail-ed1-x52c.google.com with SMTP id t3so12141670edc.7 for ; Fri, 25 Jun 2021 00:46:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=TC8WuOS2tYJ6vPTd3UnDO8bNzjZ8kU9pZ/AXWQu75Do=; b=DD0de7tAwF5ClP5aBXWRb01S3evzN5rIcGlsr2J1d2WUkfPmKlP8xdq3Plm436uS5v bwm/f0kOdxfekJ7LK7kIoTpDW/yqrkCmVhEvrAiTzZMMFmSq9uMCmEACD3YmoWn3WHc1 X0RpTYOTHmiNHmpxuyyyTj5LQd9eh149TxxLrFOwZDryhaRp4Y0XV+hS/jk5nMTfWaMr iMpJWMufAQ2z8UKMonRPj0dy5K8zvyMuh/iuTkIAwHaRMLWoX7/Q4aQ3VNG8RY8g2H4r tEz2LswIfJUEx8/k71U6sGBYofrAEh0NjonksX8mW89xwV7x0d/5gku+DB1XV4TcPfmk rs9A== X-Gm-Message-State: AOAM533WNYOzulmdk4wXvk+vYOf2AEJpfjtBwjCMvZ4t2vXFBtywqM52 oeKHKdCYcZ5ZgBUX5iKjZ71Z5WXNobtABJxB2hc= X-Google-Smtp-Source: ABdhPJz6ry9+IH/wQTNC4C5dwDZ9NkMKnTSPsakgh5UKSCDM/qBX7kSomav9U5IArpmNdqzG8PCB0p0appFntndGHck= X-Received: by 2002:a05:6402:2913:: with SMTP id ee19mr12547205edb.361.1624607190390; Fri, 25 Jun 2021 00:46:30 -0700 (PDT) MIME-Version: 1.0 References: <92db3776-af59-fa20-483b-aa67b17d0751@gmail.com> <47d06c821a53f5bd2246f0fca2c9a693bff6b882.camel@redhat.com> <3a5ea83c-0d91-d123-f537-f8f1223df890@gmail.com> <59693cea-164e-bf51-d62a-096edcdb5218@gmail.com> <159b3e62-c05d-80f7-8250-b4edad76919f@gmail.com> In-Reply-To: From: Richard Biener Date: Fri, 25 Jun 2021 09:46:19 +0200 Message-ID: Subject: Re: [PING][PATCH 9/13] v2 Use new per-location warning APIs in LTO To: Martin Sebor Cc: gcc-patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.9 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 autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Jun 2021 07:46:33 -0000 On Thu, Jun 24, 2021 at 5:27 PM Martin Sebor wrote: > > On 6/24/21 3:32 AM, Richard Biener wrote: > > On Mon, Jun 21, 2021 at 11:55 PM Martin Sebor via Gcc-patches > > wrote: > >> > >> Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-June/571980.html > >> > >> Looking for a review of the LTO changes to switch TREE_NO_WARNING to > >> the suppress_warning() API. > > > > Hmm, since the warning suppressions are on location ad-hoc data the appropriate > > thing is to adjust the location streaming and that part seems to be missing? > > > > So what you now stream is simply the "everything" fallback, correct? > > > > In particular: > > > > else > > - bp_pack_value (bp, TREE_NO_WARNING (expr), 1); > > + /* FIXME: pack all warning bits. */ > > + bp_pack_value (bp, warning_suppressed_p (expr), 1); > > > > this looks like a wrong comment in that light. > > Yes, this is just a fallback. I haven't thought about how to handle > the FIXME yet but from your comment it sounds like this code might > stay the same (or maybe even go back to streaming the flag directly) > and the nowarn_spec_t bitmap should be streamed elsewhere? Yes, since the bitmap is per location it should be streamed alongside locations in lto_{input,output}_location. > > > > - else > > - compare_values (TREE_NO_WARNING); > > + else if (t1->base.nowarning_flag != t2->base.nowarning_flag) > > + return false; > > > > uh. Can you keep sth like TREE_NO_WARNING_RAW or so? > > The flag is used directly in fold-const.c and cp/module.cc so > this would be in keeping with that, but I also don't mind adding > a macro for it. My only concern is with macro getting used to > inadvertently bypass the API. There's precedent with other _RAW accessors, it should be clear that it bypasses accessors and thus review should easily spot inadverted uses. Richard. > Martin > > > > > Thanks, > > Richard. > > > >> On 6/4/21 3:43 PM, Martin Sebor wrote: > >>> The attached patch replaces the uses of TREE_NO_WARNING in the LTO > >>> front end with the new suppress_warning() API. It adds a couple of > >>> FIXMEs that I plan to take care of in a follow up. > >> >