* [COMMITTED] Do not call range_on_path_entry for SSAs defined within the path
@ 2021-10-14 12:21 Aldy Hernandez
2021-10-15 7:51 ` Christophe LYON
0 siblings, 1 reply; 3+ messages in thread
From: Aldy Hernandez @ 2021-10-14 12:21 UTC (permalink / raw)
To: GCC patches
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;
+}
--
2.31.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [COMMITTED] Do not call range_on_path_entry for SSAs defined within the path
2021-10-14 12:21 [COMMITTED] Do not call range_on_path_entry for SSAs defined within the path Aldy Hernandez
@ 2021-10-15 7:51 ` Christophe LYON
2021-10-15 11:51 ` Aldy Hernandez
0 siblings, 1 reply; 3+ messages in thread
From: Christophe LYON @ 2021-10-15 7:51 UTC (permalink / raw)
To: gcc-patches
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [COMMITTED] Do not call range_on_path_entry for SSAs defined within the path
2021-10-15 7:51 ` Christophe LYON
@ 2021-10-15 11:51 ` Aldy Hernandez
0 siblings, 0 replies; 3+ messages in thread
From: Aldy Hernandez @ 2021-10-15 11:51 UTC (permalink / raw)
To: Christophe LYON; +Cc: gcc-patches
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
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-15 11:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-14 12:21 [COMMITTED] Do not call range_on_path_entry for SSAs defined within the path Aldy Hernandez
2021-10-15 7:51 ` Christophe LYON
2021-10-15 11:51 ` Aldy Hernandez
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).