[COMMITTED] Denormalize VR_VARYING to VR_RANGE before passing it to set_range_info_raw.
Message ID | 20220501121357.942748-1-aldyh@redhat.com |
---|---|
State | Committed |
Commit | 75bbc3da3e5f75f683fa33e309045c582efd20eb |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B0931385843E for <patchwork@sourceware.org>; Sun, 1 May 2022 12:15:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B0931385843E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1651407334; bh=9IYfkACR+c9ntXSTtnedqdGO8M2gZ/s0wPnukha/96g=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=MS16qCKLOZZ+h7WO4II+SaTZFyi0ED9WkBhuXywG5Hl76/U/rtDxoe4UaBua/UAAS areTJCuVfOANQ9UTgIB9ggsxA+oA4BoXw4d+alRFJquMLTys3WBI5iYE7dIsv8TE8Y 6e7zy/SEWTU5UbtVlsicGWJmkAMXvG+Q5SP/wLK0= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 3447C3858D28 for <gcc-patches@gcc.gnu.org>; Sun, 1 May 2022 12:15:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3447C3858D28 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-295-cmfTx1kgOuyJBHp3C1L0BQ-1; Sun, 01 May 2022 08:15:04 -0400 X-MC-Unique: cmfTx1kgOuyJBHp3C1L0BQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 9BE8D85A5BE for <gcc-patches@gcc.gnu.org>; Sun, 1 May 2022 12:15:04 +0000 (UTC) Received: from abulafia (unknown [10.39.192.84]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4A41040C1244; Sun, 1 May 2022 12:15:04 +0000 (UTC) Received: from abulafia (localhost [127.0.0.1]) by abulafia (8.17.1/8.17.1) with ESMTPS id 241CF2C7942838 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Sun, 1 May 2022 14:15:02 +0200 Received: (from aldyh@localhost) by abulafia (8.17.1/8.17.1/Submit) id 241CF2FP942773; Sun, 1 May 2022 14:15:02 +0200 To: GCC patches <gcc-patches@gcc.gnu.org> Subject: [COMMITTED] Denormalize VR_VARYING to VR_RANGE before passing it to set_range_info_raw. Date: Sun, 1 May 2022 14:13:57 +0200 Message-Id: <20220501121357.942748-1-aldyh@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Aldy Hernandez via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Aldy Hernandez <aldyh@redhat.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
[COMMITTED] Denormalize VR_VARYING to VR_RANGE before passing it to set_range_info_raw.
|
|
Commit Message
Aldy Hernandez
May 1, 2022, 12:13 p.m. UTC
We are ICEing in set_range_info_raw because value_range_kind cannot be VR_VARYING, since SSA_NAME_RANGE_TYPE can only hold VR_RANGE / VR_ANTI_RANGE. Most of the time setting a VR_VARYING as a global range makes no sense. However, we can have a range spanning the entire domain (VR_RANGE of [MIN,MAX] which is essentially a VR_VARYING), if the nonzero bits are set. This was working before because set_range_info_raw allows setting VR_RANGE of [MIN, MAX]. However, when going through an irange, we normalize this to a VR_VARYING, thus causing the ICE. It's interesting that other calls to set_range_info with an irange haven't triggered this. One solution would be to just ignore VR_VARYING and bail, since set_range_info* is really an update of the current range semantic wise. After all, we keep the nonzero bits which provide additional info. But this would be a change in behavior, so not suitable until after GCC 12 is released. So in order to keep with current behavior we can just denormalize the varying to VR_RANGE. Tested on x86-64 Linux. PR tree-optimization/105432 gcc/ChangeLog: * tree-ssanames.cc (set_range_info): Denormalize VR_VARYING to VR_RANGE before passing a piecewise range to set_range_info_raw. --- gcc/tree-ssanames.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
Comments
On Sun, May 1, 2022 at 2:15 PM Aldy Hernandez via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > We are ICEing in set_range_info_raw because value_range_kind cannot be > VR_VARYING, since SSA_NAME_RANGE_TYPE can only hold VR_RANGE / > VR_ANTI_RANGE. Most of the time setting a VR_VARYING as a global > range makes no sense. However, we can have a range spanning the > entire domain (VR_RANGE of [MIN,MAX] which is essentially a > VR_VARYING), if the nonzero bits are set. > > This was working before because set_range_info_raw allows setting > VR_RANGE of [MIN, MAX]. However, when going through an irange, we > normalize this to a VR_VARYING, thus causing the ICE. It's > interesting that other calls to set_range_info with an irange haven't > triggered this. > > One solution would be to just ignore VR_VARYING and bail, since > set_range_info* is really an update of the current range semantic > wise. After all, we keep the nonzero bits which provide additional > info. But this would be a change in behavior, so not suitable until > after GCC 12 is released. So in order to keep with current behavior > we can just denormalize the varying to VR_RANGE. But there's no point in storing such info - shouldn't the function simply clear the range and return? Or at least avoid allocating a new range -info and just return if none is associated with the SSA name at the moment? > Tested on x86-64 Linux. > > PR tree-optimization/105432 > > gcc/ChangeLog: > > * tree-ssanames.cc (set_range_info): Denormalize VR_VARYING to > VR_RANGE before passing a piecewise range to set_range_info_raw. > --- > gcc/tree-ssanames.cc | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/gcc/tree-ssanames.cc b/gcc/tree-ssanames.cc > index c957597af4f..05536cd2f74 100644 > --- a/gcc/tree-ssanames.cc > +++ b/gcc/tree-ssanames.cc > @@ -395,8 +395,17 @@ set_range_info (tree name, enum value_range_kind range_type, > { > gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); > > - /* A range of the entire domain is really no range at all. */ > tree type = TREE_TYPE (name); > + if (range_type == VR_VARYING) > + { > + /* SSA_NAME_RANGE_TYPE can only hold a VR_RANGE or > + VR_ANTI_RANGE. Denormalize VR_VARYING to VR_RANGE. */ > + range_type = VR_RANGE; > + gcc_checking_assert (min == wi::min_value (type)); > + gcc_checking_assert (max == wi::max_value (type)); > + } > + > + /* A range of the entire domain is really no range at all. */ > if (min == wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type)) > && max == wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type))) > { > -- > 2.35.1 >
On Mon, May 2, 2022 at 8:34 AM Richard Biener <richard.guenther@gmail.com> wrote: > > On Sun, May 1, 2022 at 2:15 PM Aldy Hernandez via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > We are ICEing in set_range_info_raw because value_range_kind cannot be > > VR_VARYING, since SSA_NAME_RANGE_TYPE can only hold VR_RANGE / > > VR_ANTI_RANGE. Most of the time setting a VR_VARYING as a global > > range makes no sense. However, we can have a range spanning the > > entire domain (VR_RANGE of [MIN,MAX] which is essentially a > > VR_VARYING), if the nonzero bits are set. > > > > This was working before because set_range_info_raw allows setting > > VR_RANGE of [MIN, MAX]. However, when going through an irange, we > > normalize this to a VR_VARYING, thus causing the ICE. It's > > interesting that other calls to set_range_info with an irange haven't > > triggered this. > > > > One solution would be to just ignore VR_VARYING and bail, since > > set_range_info* is really an update of the current range semantic > > wise. After all, we keep the nonzero bits which provide additional > > info. But this would be a change in behavior, so not suitable until > > after GCC 12 is released. So in order to keep with current behavior > > we can just denormalize the varying to VR_RANGE. > > But there's no point in storing such info - shouldn't the function simply > clear the range and return? Or at least avoid allocating a new range > -info and just return if none is associated with the SSA name at the moment? Yup. That's what the code block immediately following will do: clear the range (and free the storage), but only if the nonzero bits are also clear. This duality of ranges with nonzero bits and ranges is a bit annoying. I have code overhauling this for when we provide a way to store global (multi) ranges later this cycle. Nonzero bits will live in the range itself, and updating the global range / nonzero bits will have one entry point that takes a range. Also, nonzero bits and ranges will be kept up to date wrt each other automatically. Aldy
diff --git a/gcc/tree-ssanames.cc b/gcc/tree-ssanames.cc index c957597af4f..05536cd2f74 100644 --- a/gcc/tree-ssanames.cc +++ b/gcc/tree-ssanames.cc @@ -395,8 +395,17 @@ set_range_info (tree name, enum value_range_kind range_type, { gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); - /* A range of the entire domain is really no range at all. */ tree type = TREE_TYPE (name); + if (range_type == VR_VARYING) + { + /* SSA_NAME_RANGE_TYPE can only hold a VR_RANGE or + VR_ANTI_RANGE. Denormalize VR_VARYING to VR_RANGE. */ + range_type = VR_RANGE; + gcc_checking_assert (min == wi::min_value (type)); + gcc_checking_assert (max == wi::max_value (type)); + } + + /* A range of the entire domain is really no range at all. */ if (min == wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type)) && max == wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type))) {