Message ID | 20211014122143.227902-1-aldyh@redhat.com |
---|---|
State | Committed |
Commit | a311163fd81babd6116e2856f4551c3ca13d8914 |
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 2D15B3858020 for <patchwork@sourceware.org>; Thu, 14 Oct 2021 12:22:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2D15B3858020 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1634214179; bh=tAHR8FZ6hqoG5cwbC/EBMIwPmGpEuFqGIDXS1G2lasM=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=YhLQs6BbtiLWXw1KtOyf74dpkmlOax7pMI1XR/a5QN4x7PHrhmJdN+W5czloX0ROu 6sbHv5MAJhhOwUTQoXu5mHFS5lni/wLexKieAne+LLj5qiOCy/z9IDIwxbAp/LuVT6 6OUY6XLTy35B26w5mqYqoqj+akM9zoiDVoSiijQI= 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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 90E6C3858015 for <gcc-patches@gcc.gnu.org>; Thu, 14 Oct 2021 12:22:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 90E6C3858015 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-601-eX2dYI0YPc6Jj07ieXGhxw-1; Thu, 14 Oct 2021 08:22:26 -0400 X-MC-Unique: eX2dYI0YPc6Jj07ieXGhxw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B2AD310247AC for <gcc-patches@gcc.gnu.org>; Thu, 14 Oct 2021 12:22:25 +0000 (UTC) Received: from abulafia.quesejoda.com (unknown [10.39.193.225]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4671D5DF56; Thu, 14 Oct 2021 12:21:52 +0000 (UTC) Received: from abulafia.quesejoda.com (localhost [127.0.0.1]) by abulafia.quesejoda.com (8.16.1/8.15.2) with ESMTPS id 19ECLn0v227952 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 14 Oct 2021 14:21:49 +0200 Received: (from aldyh@localhost) by abulafia.quesejoda.com (8.16.1/8.16.1/Submit) id 19ECLnBV227951; Thu, 14 Oct 2021 14:21:49 +0200 To: GCC patches <gcc-patches@gcc.gnu.org> Subject: [COMMITTED] Do not call range_on_path_entry for SSAs defined within the path Date: Thu, 14 Oct 2021 14:21:43 +0200 Message-Id: <20211014122143.227902-1-aldyh@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" X-Spam-Status: No, score=-13.1 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, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP 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] Do not call range_on_path_entry for SSAs defined within the path
|
|
Commit Message
Aldy Hernandez
Oct. 14, 2021, 12:21 p.m. UTC
In the path solver, when requesting the range of an SSA for which we know nothing, we ask the ranger for the range incoming to the path. We do this by asking for all the incoming ranges to the path entry block and unioning them. The problem here is that we're asking for a range on path entry for an SSA which *is* defined in the path, but for which we know nothing about: some_global.1_2 = some_global; _3 = (char) some_global.1_2; This request is causing us to ask for range_on_edge of _3 on the incoming edges to the path. This is a bit of nonsensical request because _3 isn't live on entry to the path, so ranger correctly returns UNDEFINED. The proper thing is to avoid asking this in the first place. I have added a relevant assert, since it doesn't make sense to call range_on_path_entry for SSAs defined within the path. Tested on x86-64 Linux. PR 102736 gcc/ChangeLog: PR tree/optimization/102736 * gimple-range-path.cc (path_range_query::range_on_path_entry): Assert that the requested range is defined outside the path. (path_range_query::ssa_range_in_phi): Do not call range_on_path_entry for SSA names that are defined within the path. gcc/testsuite/ChangeLog: * gcc.dg/tree-ssa/pr102736.c: New test. --- gcc/gimple-range-path.cc | 6 +++++- gcc/testsuite/gcc.dg/tree-ssa/pr102736.c | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr102736.c
Comments
On 14/10/2021 14:21, Aldy Hernandez via Gcc-patches wrote: > In the path solver, when requesting the range of an SSA for which we > know nothing, we ask the ranger for the range incoming to the path. > We do this by asking for all the incoming ranges to the path entry > block and unioning them. > > The problem here is that we're asking for a range on path entry for an > SSA which *is* defined in the path, but for which we know nothing > about: > > some_global.1_2 = some_global; > _3 = (char) some_global.1_2; > > This request is causing us to ask for range_on_edge of _3 on the > incoming edges to the path. This is a bit of nonsensical request > because _3 isn't live on entry to the path, so ranger correctly > returns UNDEFINED. The proper thing is to avoid asking this in the > first place. > > I have added a relevant assert, since it doesn't make sense to call > range_on_path_entry for SSAs defined within the path. > > Tested on x86-64 Linux. > > PR 102736 > > gcc/ChangeLog: > > PR tree/optimization/102736 > * gimple-range-path.cc (path_range_query::range_on_path_entry): > Assert that the requested range is defined outside the path. > (path_range_query::ssa_range_in_phi): Do not call > range_on_path_entry for SSA names that are defined within the > path. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/pr102736.c: New test. > --- > gcc/gimple-range-path.cc | 6 +++++- > gcc/testsuite/gcc.dg/tree-ssa/pr102736.c | 21 +++++++++++++++++++++ > 2 files changed, 26 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr102736.c > > diff --git a/gcc/gimple-range-path.cc b/gcc/gimple-range-path.cc > index 422abfddb8f..694271306a7 100644 > --- a/gcc/gimple-range-path.cc > +++ b/gcc/gimple-range-path.cc > @@ -134,6 +134,7 @@ path_range_query::defined_outside_path (tree name) > void > path_range_query::range_on_path_entry (irange &r, tree name) > { > + gcc_checking_assert (defined_outside_path (name)); > int_range_max tmp; > basic_block entry = entry_bb (); > bool changed = false; > @@ -258,7 +259,10 @@ path_range_query::ssa_range_in_phi (irange &r, gphi *phi) > // Using both the range on entry to the path, and the > // range on this edge yields significantly better > // results. > - range_on_path_entry (r, arg); > + if (defined_outside_path (arg)) > + range_on_path_entry (r, arg); > + else > + r.set_varying (TREE_TYPE (name)); > m_ranger.range_on_edge (tmp, e_in, arg); > r.intersect (tmp); > return; > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr102736.c b/gcc/testsuite/gcc.dg/tree-ssa/pr102736.c > new file mode 100644 > index 00000000000..7e556f01a86 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr102736.c > @@ -0,0 +1,21 @@ > +// { dg-do run } > +// { dg-options "-O1 -ftree-vrp" } > + > +int a, b = -1, c; > +int d = 1; > +static inline char e(char f, int g) { return g ? f : 0; } > +static inline char h(char f) { return f < a ? f : f < a; } > +static inline unsigned char i(unsigned char f, int g) { return g ? f : f > g; } > +void j() { > +L: > + c = e(1, i(h(b), d)); > + if (b) > + return; > + goto L; > +} > +int main() { > + j(); > + if (c != 1) > + __builtin_abort (); > + return 0; > +} Hi, The new test fails at execution on arm / aarch64, not sure if you are aware of that already? Thanks, Christophe
It's been fixed on trunk. Aldy On Fri, Oct 15, 2021, 09:52 Christophe LYON via Gcc-patches < gcc-patches@gcc.gnu.org> wrote: > > On 14/10/2021 14:21, Aldy Hernandez via Gcc-patches wrote: > > In the path solver, when requesting the range of an SSA for which we > > know nothing, we ask the ranger for the range incoming to the path. > > We do this by asking for all the incoming ranges to the path entry > > block and unioning them. > > > > The problem here is that we're asking for a range on path entry for an > > SSA which *is* defined in the path, but for which we know nothing > > about: > > > > some_global.1_2 = some_global; > > _3 = (char) some_global.1_2; > > > > This request is causing us to ask for range_on_edge of _3 on the > > incoming edges to the path. This is a bit of nonsensical request > > because _3 isn't live on entry to the path, so ranger correctly > > returns UNDEFINED. The proper thing is to avoid asking this in the > > first place. > > > > I have added a relevant assert, since it doesn't make sense to call > > range_on_path_entry for SSAs defined within the path. > > > > Tested on x86-64 Linux. > > > > PR 102736 > > > > gcc/ChangeLog: > > > > PR tree/optimization/102736 > > * gimple-range-path.cc (path_range_query::range_on_path_entry): > > Assert that the requested range is defined outside the path. > > (path_range_query::ssa_range_in_phi): Do not call > > range_on_path_entry for SSA names that are defined within the > > path. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.dg/tree-ssa/pr102736.c: New test. > > --- > > gcc/gimple-range-path.cc | 6 +++++- > > gcc/testsuite/gcc.dg/tree-ssa/pr102736.c | 21 +++++++++++++++++++++ > > 2 files changed, 26 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr102736.c > > > > diff --git a/gcc/gimple-range-path.cc b/gcc/gimple-range-path.cc > > index 422abfddb8f..694271306a7 100644 > > --- a/gcc/gimple-range-path.cc > > +++ b/gcc/gimple-range-path.cc > > @@ -134,6 +134,7 @@ path_range_query::defined_outside_path (tree name) > > void > > path_range_query::range_on_path_entry (irange &r, tree name) > > { > > + gcc_checking_assert (defined_outside_path (name)); > > int_range_max tmp; > > basic_block entry = entry_bb (); > > bool changed = false; > > @@ -258,7 +259,10 @@ path_range_query::ssa_range_in_phi (irange &r, gphi > *phi) > > // Using both the range on entry to the path, and the > > // range on this edge yields significantly better > > // results. > > - range_on_path_entry (r, arg); > > + if (defined_outside_path (arg)) > > + range_on_path_entry (r, arg); > > + else > > + r.set_varying (TREE_TYPE (name)); > > m_ranger.range_on_edge (tmp, e_in, arg); > > r.intersect (tmp); > > return; > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr102736.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr102736.c > > new file mode 100644 > > index 00000000000..7e556f01a86 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr102736.c > > @@ -0,0 +1,21 @@ > > +// { dg-do run } > > +// { dg-options "-O1 -ftree-vrp" } > > + > > +int a, b = -1, c; > > +int d = 1; > > +static inline char e(char f, int g) { return g ? f : 0; } > > +static inline char h(char f) { return f < a ? f : f < a; } > > +static inline unsigned char i(unsigned char f, int g) { return g ? f : > f > g; } > > +void j() { > > +L: > > + c = e(1, i(h(b), d)); > > + if (b) > > + return; > > + goto L; > > +} > > +int main() { > > + j(); > > + if (c != 1) > > + __builtin_abort (); > > + return 0; > > +} > > Hi, > > > The new test fails at execution on arm / aarch64, not sure if you are > aware of that already? > > > Thanks, > > Christophe > > >
diff --git a/gcc/gimple-range-path.cc b/gcc/gimple-range-path.cc index 422abfddb8f..694271306a7 100644 --- a/gcc/gimple-range-path.cc +++ b/gcc/gimple-range-path.cc @@ -134,6 +134,7 @@ path_range_query::defined_outside_path (tree name) void path_range_query::range_on_path_entry (irange &r, tree name) { + gcc_checking_assert (defined_outside_path (name)); int_range_max tmp; basic_block entry = entry_bb (); bool changed = false; @@ -258,7 +259,10 @@ path_range_query::ssa_range_in_phi (irange &r, gphi *phi) // Using both the range on entry to the path, and the // range on this edge yields significantly better // results. - range_on_path_entry (r, arg); + if (defined_outside_path (arg)) + range_on_path_entry (r, arg); + else + r.set_varying (TREE_TYPE (name)); m_ranger.range_on_edge (tmp, e_in, arg); r.intersect (tmp); return; diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr102736.c b/gcc/testsuite/gcc.dg/tree-ssa/pr102736.c new file mode 100644 index 00000000000..7e556f01a86 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr102736.c @@ -0,0 +1,21 @@ +// { dg-do run } +// { dg-options "-O1 -ftree-vrp" } + +int a, b = -1, c; +int d = 1; +static inline char e(char f, int g) { return g ? f : 0; } +static inline char h(char f) { return f < a ? f : f < a; } +static inline unsigned char i(unsigned char f, int g) { return g ? f : f > g; } +void j() { +L: + c = e(1, i(h(b), d)); + if (b) + return; + goto L; +} +int main() { + j(); + if (c != 1) + __builtin_abort (); + return 0; +}