From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x52b.google.com (mail-pg1-x52b.google.com [IPv6:2607:f8b0:4864:20::52b]) by sourceware.org (Postfix) with ESMTPS id E13013857B9A for ; Fri, 5 Aug 2022 15:46:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E13013857B9A Received: by mail-pg1-x52b.google.com with SMTP id l64so3045428pge.0 for ; Fri, 05 Aug 2022 08:46:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=3toxkLci1yy9l3qlOfW7i4H2ZPCI88LyavK+zsmLIU4=; b=bhMtAmYRXdiscVPCEM9dnSFLpoVCQkeFz4RvyAW8B48xEF8dSPfJ3j4bcWSK+ILsL+ eGmpY0J07Tdcw2clSCwmBJifzw764kiZ68nHHC89f8oWfYuoMj5/R/q01FEgVuHaxzD5 DFoYkep84m75T3a5aZKMUuRoq8H4pxnyqbO39iteZTzmgHYiZwhcaI4oY1dI/453GwiC tYkp9DASYZ64oRjHQTHmx9X1uluHAzVWic4PCEe+JQ+t4Xkts7JpmNfr1nspECDlJrOC 2u4ctB+SJVShLfgwBUe+sq1yJFrjuyrsIUnmza2LoM4ruZ8/qNeiOYylsNc5RXyCXRz/ v+Hw== X-Gm-Message-State: ACgBeo3ZNizeanEKvgcoJaZuQ8gBjIKZiN47ddPHCQncoTppam+lRUBJ tXV3jXQvIIa0Q5qUhQxj8lk= X-Google-Smtp-Source: AA6agR620H9R03qjw5Bsl8GSa2nQ8ZYD6JC7MP5kF1oSuZbt/uScS+5X8NZw+wEVn0aO0Q+AnDBX5w== X-Received: by 2002:a05:6a00:1a0c:b0:528:6baa:a2e3 with SMTP id g12-20020a056a001a0c00b005286baaa2e3mr7476796pfv.27.1659714391691; Fri, 05 Aug 2022 08:46:31 -0700 (PDT) Received: from [172.31.0.204] (c-73-98-188-51.hsd1.ut.comcast.net. [73.98.188.51]) by smtp.gmail.com with ESMTPSA id h7-20020a170902f54700b0016db1b67fb9sm3154202plf.224.2022.08.05.08.46.30 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 05 Aug 2022 08:46:31 -0700 (PDT) Message-ID: <4fc37a51-ef49-07d8-d927-d8a1f1ce2eeb@gmail.com> Date: Fri, 5 Aug 2022 09:46:29 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH] Adjust backwards threader costing of PHIs Content-Language: en-US To: Richard Biener , gcc-patches@gcc.gnu.org References: <20220805135803.8DAE2133B5@imap2.suse-dmz.suse.de> From: Jeff Law In-Reply-To: <20220805135803.8DAE2133B5@imap2.suse-dmz.suse.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, NICE_REPLY_A, 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Aug 2022 15:46:34 -0000 On 8/5/2022 7:58 AM, Richard Biener wrote: > The following adjusts the costing of PHIs to match how I understand > the comment and maybe the original intent. The will be no > non-degenerate PHI nodes remaining on the threaded path but when there > are alternate path exits PHI nodes at their destinations will likely > require extra copies on those edges and that's what we want to account > for. > > Jeff added this code long time ago and at least the special-casing > of (just) m_name does not make sense anymore. > > Unfortunately this tweaks heuristics enough to make the threader > thread an unfortunate path in libgomp/team.c so that > -Walloca-larger-than diagnoses an allocation of -1 elements. I'm > not 100% sure this condition is impossible so I've added a guard > not allocating zero or "less" stack. There's also an uninit > diagnostic in opts.cc about best_math::m_best_candidate_len that > looks like a false positive but all but this member are initialized > so the patch initializes this one as well to avoid this false > positive. > > I have yet to analyze some fallout as well: > > FAIL: gcc.dg/uninit-pred-9_b.c bogus warning (test for bogus messages, line 20) > FAIL: gcc.dg/tree-ssa/phi_on_compare-4.c scan-tree-dump-times dom2 "Removing bas > ic block" 1 > FAIL: gcc.dg/tree-ssa/pr59597.c scan-tree-dump-times threadfull1 "Registering jump thread" 4 > FAIL: gcc.dg/tree-ssa/pr61839_2.c scan-tree-dump-times evrp "%" 0 > FAIL: gcc.dg/tree-ssa/pr61839_2.c scan-tree-dump-times evrp "972195717 % " 0 > FAIL: gcc.dg/tree-ssa/pr61839_2.c scan-tree-dump-times evrp "972195717 / " 0 > FAIL: gcc.dg/tree-ssa/pr61839_2.c scan-tree-dump-times evrp "__builtin_abort" 1 > > the early threading fails are because we now account for the mid-exit > PHI copies and early threading has a limit of a single copied insn. > But this testcase was never about threading but VRP which seemingly > regressed meanwhile... > > Bootstrapped and tested (with the above FAILs) on > x86_64-unknown-linux-gnu. > > Any comments besides the FAILout? I don't recall adding that code, but I did find it in the archives. https://gcc.gnu.org/pipermail/gcc-patches/2016-March/443452.html But even with that and reviewing the PR, I still don't remember much about this particular chunk of code.  I do recall that I was never actually happy with pr69196 state and that's why we've kept it open all these years. It doesn't look like tree-ssa/pr69196 has regressed, so if you're happy with the patch, I've got no objections. jeff