Message ID | cb99c648-b359-9b0f-29af-1292cf7d0bdd@suse.cz |
---|---|
State | New |
Headers | show |
Series | IPA: compare VRP types. | expand |
On October 15, 2020 5:04:11 PM GMT+02:00, "Martin Liška" <mliska@suse.cz> wrote: >Hello. > >As mentioned in the PR, since 74ca1c01d02e548f32aa26f9a887dcc0730703fb >we wrongly >merge a pair of VRP ranges with a different TREE_TYPE (in the PR one is >char and second >one is short). Then we merge 2 ranges and make an anti-range and bad >things happen. > >Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > >Ready to be installed? One check should suffice. Min ant max of a range should have compatible types. >Thanks, >Martin > >gcc/ChangeLog: > > PR ipa/97404 > * ipa-prop.c (struct ipa_vr_ggc_hash_traits): > Compare types of VRP min and max as we can merge ranges of > different types. > >gcc/testsuite/ChangeLog: > > PR ipa/97404 > * gcc.c-torture/execute/pr97404.c: New test. >--- > gcc/ipa-prop.c | 6 +++- > gcc/testsuite/gcc.c-torture/execute/pr97404.c | 28 +++++++++++++++++++ > 2 files changed, 33 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr97404.c > >diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c >index 63c652f4ffc..3d0ebefa4aa 100644 >--- a/gcc/ipa-prop.c >+++ b/gcc/ipa-prop.c >@@ -123,7 +123,11 @@ struct ipa_vr_ggc_hash_traits : public >ggc_cache_remove <value_range *> > static bool > equal (const value_range *a, const value_range *b) > { >- return a->equal_p (*b); >+ return (a->equal_p (*b) >+ && types_compatible_p (TREE_TYPE (a->min ()), >+ TREE_TYPE (b->min ())) >+ && types_compatible_p (TREE_TYPE (a->max ()), >+ TREE_TYPE (b->max ()))); > } > static const bool empty_zero_p = true; > static void >diff --git a/gcc/testsuite/gcc.c-torture/execute/pr97404.c >b/gcc/testsuite/gcc.c-torture/execute/pr97404.c >new file mode 100644 >index 00000000000..7e5ce231725 >--- /dev/null >+++ b/gcc/testsuite/gcc.c-torture/execute/pr97404.c >@@ -0,0 +1,28 @@ >+/* PR ipa/97404 */ >+/* { dg-additional-options "-fno-inline" } */ >+ >+char a, b; >+long c; >+short d, e; >+long *f = &c; >+int g; >+char h(signed char i) { return 0; } >+static short j(short i, int k) { return i < 0 ? 0 : i >> k; } >+void l(void); >+void m(void) >+{ >+ e = j(d | 9766, 11); >+ *f = e; >+} >+void l(void) >+{ >+ a = 5 | g; >+ b = h(a); >+} >+int main() >+{ >+ m(); >+ if (c != 4) >+ __builtin_abort(); >+ return 0; >+}
On Thu, Oct 15 2020, Martin Liška wrote: > Hello. > > As mentioned in the PR, since 74ca1c01d02e548f32aa26f9a887dcc0730703fb we wrongly > merge a pair of VRP ranges with a different TREE_TYPE (in the PR one is char and second > one is short). Then we merge 2 ranges and make an anti-range and bad things happen. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin > > gcc/ChangeLog: > > PR ipa/97404 > * ipa-prop.c (struct ipa_vr_ggc_hash_traits): > Compare types of VRP min and max as we can merge ranges of > different types. > > gcc/testsuite/ChangeLog: > > PR ipa/97404 > * gcc.c-torture/execute/pr97404.c: New test. > --- > gcc/ipa-prop.c | 6 +++- > gcc/testsuite/gcc.c-torture/execute/pr97404.c | 28 +++++++++++++++++++ > 2 files changed, 33 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr97404.c > > diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c > index 63c652f4ffc..3d0ebefa4aa 100644 > --- a/gcc/ipa-prop.c > +++ b/gcc/ipa-prop.c > @@ -123,7 +123,11 @@ struct ipa_vr_ggc_hash_traits : public ggc_cache_remove <value_range *> > static bool > equal (const value_range *a, const value_range *b) > { > - return a->equal_p (*b); > + return (a->equal_p (*b) > + && types_compatible_p (TREE_TYPE (a->min ()), > + TREE_TYPE (b->min ())) > + && types_compatible_p (TREE_TYPE (a->max ()), > + TREE_TYPE (b->max ()))); I think the best check is: types_compatible_p (a->type (), b->type ()) ...which internally checks just the minima, but I understand the intention is that the types are the same or at least compatible. OK with that change. Thanks! Martin
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c index 63c652f4ffc..3d0ebefa4aa 100644 --- a/gcc/ipa-prop.c +++ b/gcc/ipa-prop.c @@ -123,7 +123,11 @@ struct ipa_vr_ggc_hash_traits : public ggc_cache_remove <value_range *> static bool equal (const value_range *a, const value_range *b) { - return a->equal_p (*b); + return (a->equal_p (*b) + && types_compatible_p (TREE_TYPE (a->min ()), + TREE_TYPE (b->min ())) + && types_compatible_p (TREE_TYPE (a->max ()), + TREE_TYPE (b->max ()))); } static const bool empty_zero_p = true; static void diff --git a/gcc/testsuite/gcc.c-torture/execute/pr97404.c b/gcc/testsuite/gcc.c-torture/execute/pr97404.c new file mode 100644 index 00000000000..7e5ce231725 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr97404.c @@ -0,0 +1,28 @@ +/* PR ipa/97404 */ +/* { dg-additional-options "-fno-inline" } */ + +char a, b; +long c; +short d, e; +long *f = &c; +int g; +char h(signed char i) { return 0; } +static short j(short i, int k) { return i < 0 ? 0 : i >> k; } +void l(void); +void m(void) +{ + e = j(d | 9766, 11); + *f = e; +} +void l(void) +{ + a = 5 | g; + b = h(a); +} +int main() +{ + m(); + if (c != 4) + __builtin_abort(); + return 0; +}