From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19111 invoked by alias); 28 Feb 2019 19:54:33 -0000 Mailing-List: contact gcc-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-owner@gcc.gnu.org Received: (qmail 19092 invoked by uid 89); 28 Feb 2019 19:54:32 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=zhao, Zhao, Qing, vrp 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; Thu, 28 Feb 2019 19:54:31 +0000 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E901A194A71; Thu, 28 Feb 2019 19:54:29 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-13.rdu2.redhat.com [10.10.112.13]) by smtp.corp.redhat.com (Postfix) with ESMTP id DF1941001DE8; Thu, 28 Feb 2019 19:54:28 +0000 (UTC) Subject: Re: A bug in vrp_meet? To: Qing Zhao , gcc@gcc.gnu.org, gcc Patches References: From: Jeff Law Openpgp: preference=signencrypt Message-ID: <933b52ac-d372-f9d9-792e-4166f35b41f5@redhat.com> Date: Thu, 28 Feb 2019 19:54:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2019-02/txt/msg00173.txt.bz2 On 2/28/19 10:05 AM, Qing Zhao wrote: > Hi, > > I have been debugging a runtime error caused by value range propagation. and finally located to the following gcc routine: > > vrp_meet_1 in gcc/tree-vrp.c > > ==== > /* Meet operation for value ranges. Given two value ranges VR0 and > VR1, store in VR0 a range that contains both VR0 and VR1. This > may not be the smallest possible such range. */ > > static void > vrp_meet_1 (value_range *vr0, const value_range *vr1) > { > value_range saved; > > if (vr0->type == VR_UNDEFINED) > { > set_value_range (vr0, vr1->type, vr1->min, vr1->max, vr1->equiv); > return; > } > > if (vr1->type == VR_UNDEFINED) > { > /* VR0 already has the resulting range. */ > return; > } > ==== > > In the above, when one of vr0 or vr1 is VR_UNDEFINED, the meet result of these two will be the other VALUE. > > This seems not correct to me. That's the optimistic nature of VRP. It's generally desired behavior. > > For example, the following is the located incorrect value range propagation: (portion from the dump file *.181t.dom3) > > ==== > Visiting PHI node: i_83 = PHI <_152(20), 0(22)> > Argument #0 (20 -> 10 executable) > _152: UNDEFINED > Argument #1 (22 -> 10 executable) > 0: [0, 0] > Meeting > UNDEFINED > and > [0, 0] > to > [0, 0] > Intersecting > [0, 0] > and > [0, 65535] > to > [0, 0] > ==== > > > In the above, “i_83” is defined as PHI <_152(20), 0(22)>, the 1st argument is UNDEFINED at this time(but its value range definitely is NOT [0,0]), > and the 2nd argument is 0. If it's value is undefined then it can be any value we want. We choose to make it equal to the other argument. If VRP later finds that _152 changes, then it will go back and reevaluate the PHI. That's one of the basic design principles of the optimistic propagators. > > “vrp_meet” generate a VR_RANGE with [0,0] for “i_83” based on the current algorithm. Obviously, this result VR_RANGE with [0,0] does NOT > contain the value ranges for _152. > > the result of “vrp_meet” is Not correct. and this incorrect value range result finally caused the runtime error. > > I ‘d like to modify the vrp_meet_1 as following: > > ==== > static void > vrp_meet_1 (value_range *vr0, const value_range *vr1) > { > value_range saved; > > if (vr0->type == VR_UNDEFINED) > { > /* VR0 already has the resulting range. */ > return; > } > > if (vr1->type == VR_UNDEFINED) > { > set_value_range_to_undefined (vr0) > return; > } > ==== > > let me know your opinion. > > thanks a lot for the help. I think we (Richi and I) went through this about a year ago and the conclusion was we should be looking at how you're getting into the vrp_meet with the VR_UNDEFINED. If it happens because the user's code has an undefined use, then, the consensus has been to go ahead and optimize it. If it happens for any other reason, then it's likely a bug in GCC. We had a couple of these when we made EVRP a re-usable module and started exploiting its data in the jump threader. So you need to work backwards from this point to figure out how you got here. jeff