From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 77190 invoked by alias); 28 Nov 2017 14:57:10 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 77180 invoked by uid 89); 28 Nov 2017 14:57:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=BAYES_00,KAM_NUMSUBJECT,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=refined X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 28 Nov 2017 14:57:09 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E959A356D8; Tue, 28 Nov 2017 14:57:07 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-12.rdu2.redhat.com [10.10.112.12]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1A43F60600; Tue, 28 Nov 2017 14:57:06 +0000 (UTC) Subject: Re: [PATCH] Fix PR80776 To: Richard Biener Cc: gcc-patches@gcc.gnu.org References: <3d3fbb88-52a5-4cd9-71c3-5d12cfaaf8c5@redhat.com> From: Jeff Law Message-ID: <10acb7cf-9b9e-6f1e-dbbf-bb63b8f0b4f4@redhat.com> Date: Tue, 28 Nov 2017 15:15:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg02405.txt.bz2 On 11/28/2017 02:14 AM, Richard Biener wrote: > On Mon, 27 Nov 2017, Jeff Law wrote: > >> On 11/27/2017 06:39 AM, Richard Biener wrote: >>> >>> The following avoids -Wformat-overflow false positives by teaching >>> EVRP the trick about __builtin_unreachable () "other" edges and >>> attaching range info to SSA names. EVRP does a better job in keeping >>> ranges for every SSA name from conditional info (VRP "optimizes" its >>> costly ASSERT_EXPR insertion process). >>> >>> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. >>> >>> This will also fix the testcase from PR83072 but it doesn't really >>> fix all cases I want to fix with a fix for it. OTOH it might be >>> this is enough for stage3. >>> >>> Richard. >>> >>> 2017-11-27 Richard Biener >>> >>> PR tree-optimization/80776 >>> * gimple-ssa-evrp-analyze.h (evrp_range_analyzer::set_ssa_range_info): >>> Declare. >>> * gimple-ssa-evrp-analyze.c (evrp_range_analyzer::set_ssa_range_info): >>> New function. >>> (evrp_range_analyzer::record_ranges_from_incoming_edges): >>> If the incoming edge is an effective fallthru because the other >>> edge only reaches a __builtin_unreachable () then record ranges >>> derived from the controlling condition in SSA info. >>> (evrp_range_analyzer::record_ranges_from_phis): Use set_ssa_range_info. >>> (evrp_range_analyzer::record_ranges_from_stmt): Likewise. >>> >>> * gcc.dg/pr80776-1.c: New testcase. >>> * gcc.dg/pr80776-2.c: Likewise. >> So the thing to make sure of here is that the range information we >> reflect back into the SSA_NAME actually applies everywhere the SSA_NAME >> can appear. ie, it's globally valid. >> >> This means we can't reflect anything we derive from conditionals or >> things like a *p making the range non-null back to the SSA_NAME. >> >> I'd be concerned about the change to record_ranges_from_incoming_edge. > > It's basically a copy of what VRP does when removing range assertions. > I've added the correctness check I missed and also the trick with > setting nonzero bits. > > This causes us to no longer handle the gcc.dg/pr80776-1.c case: > > : > i_4 = somerandom (); > if (i_4 < 0) > goto ; [INV] > else > goto ; [INV] > > : > __builtin_unreachable (); > > : > i.0_1 = (unsigned int) i_4; > if (i.0_1 > 999999) > goto ; [INV] > else > goto ; [INV] > > : > __builtin_unreachable (); > > : > _7 = __builtin___sprintf_chk (&number, 1, 7, "%d", i_4); > > when trying to update the SSA range info for i_4 from the > if (i.0_1 > 999999) we see the use in the dominating condition > and thus conclude we cannot update the SSA range info like we want. So the unreachables add a twist. If we assume the unreachables are in fact unreachable, then ISTM we can use the conditionals to derive refined ranges on the other edge. The implications of the unreachable didn't really sink in until just now. > > ifcombine also doesn't merge the tests because I think it gets > confused by the __builtin_unreachable (). ifcombine also runs after > VRP1 which gets rid of the __builtin_unreachable ()s. Yea. As you know I was just poking at a case yesterday where ifcombine worked sub-optimally and its placement was inconvenient... But given the particular case I was looking at it made more sense to simplify the IL prior to ifcombine. > > I think for GCC 9 we may want to experiment with moving ifcombine > before VRP1 and handling if-chains with __builtin_unreachable ()s. Worth investigation. Jeff