On Fri, Mar 19, 2021 at 4:17 PM Jakub Jelinek <ja...@redhat.com> wrote:
>
> On Fri, Mar 19, 2021 at 10:01:50AM +0100, Aldy Hernandez via Gcc-patches
> wrote:
> > --- a/gcc/value-range.cc
> > +++ b/gcc/value-range.cc
> > @@ -184,12 +184,46 @@ irange::irange_set (tree min, tree max)
> > verify_range ();
> > }
> >
> > +void
> > +irange::irange_set_1bit_anti_range (tree min, tree max)
> > +{
> > + tree type = TREE_TYPE (min);
> > + gcc_checking_assert (TYPE_PRECISION (type) == 1);
> > +
> > + if (operand_equal_p (min, max))
> > + {
> > + // Since these are 1-bit quantities, they can only be [MIN,MIN]
> > + // or [MAX,MAX].
> > + if (vrp_val_is_min (min))
> > + min = max = vrp_val_max (type);
> > + else
> > + min = max = vrp_val_min (type);
> > + set (min, max);
> > + }
> > + else
> > + {
> > + // The only alternative is [MIN,MAX], which is the empty range,
> > + // but legacy code treats this as varying.
> > + gcc_checking_assert (vrp_val_is_min (min));
> > + gcc_checking_assert (vrp_val_is_max (max));
> > + set_varying (type);
>
> Doesn't this mean we handle the precision 1 ranges inconsistently with
> the precision 2+ ranges? Because irange_set_anti_range for min equal
> to minimum and max equal to maximum (i.e. VARYING) means
> m_num_ranges = 0; (empty range).
Bah. You're right.
The correct solution is to fix legacy to have ~[MIN,MAX] yield
UNDEFINED, but I've always been afraid of changing it because it's
been an existing behavior for so long. Doubly so, this late in the
release cycle.
Let's keep irange internally consistent and have it return UNDEFINED.
Besides, the legacy code won't be long lived.
>
> > @@ -291,16 +325,10 @@ irange::set (tree min, tree max, value_range_kind
> > kind)
> > set_varying (type);
> > return;
> > }
> > - else if (TYPE_PRECISION (TREE_TYPE (min)) == 1
> > - && (is_min || is_max))
> > + else if (TYPE_PRECISION (TREE_TYPE (min)) == 1)
> > {
> > - /* Non-empty boolean ranges can always be represented
> > - as a singleton range. */
> > - if (is_min)
> > - min = max = vrp_val_max (TREE_TYPE (min));
> > - else
> > - min = max = vrp_val_min (TREE_TYPE (min));
> > - kind = VR_RANGE;
> > + irange_set_1bit_anti_range (min, max);
> > + return;
>
> This is in else branch of is_min && is_max which results in
> varying, so irange_set_1bit_anti_range would not need to deal with that case
> for this caller.
I've reverted this chunk, so it shouldn't matter.
How does this look?
Aldy
From 8498df2965c64b7b2adf9f9277879a76ef9e0eba Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <al...@redhat.com>
Date: Thu, 18 Mar 2021 16:05:27 +0100
Subject: [PATCH] Handle setting of 1-bit anti-ranges uniformly.
PR tree-optimization/99296
* value-range.cc (irange::irange_set_1bit_anti_range): New.
(irange::irange_set_anti_range): Call irange_set_1bit_anti_range
* value-range.h (irange::irange_set_1bit_anti_range): New.
---
gcc/testsuite/gcc.dg/pr99296.c | 7 +++++++
gcc/value-range.cc | 31 +++++++++++++++++++++++++++++++
gcc/value-range.h | 2 ++
3 files changed, 40 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/pr99296.c
diff --git a/gcc/testsuite/gcc.dg/pr99296.c b/gcc/testsuite/gcc.dg/pr99296.c
new file mode 100644
index 00000000000..4a0b3f0c366
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr99296.c
@@ -0,0 +1,7 @@
+// { dg-do compile }
+// { dg-options "-O2 -fno-tree-bit-ccp" }
+
+struct {
+ signed a : 1;
+} b, c;
+void d() { b.a |= c.a |= 0 != 2; }
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 9c42f82a105..cd21f75909b 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -184,12 +184,43 @@ irange::irange_set (tree min, tree max)
verify_range ();
}
+void
+irange::irange_set_1bit_anti_range (tree min, tree max)
+{
+ tree type = TREE_TYPE (min);
+ gcc_checking_assert (TYPE_PRECISION (type) == 1);
+
+ if (operand_equal_p (min, max))
+ {
+ // Since these are 1-bit quantities, they can only be [MIN,MIN]
+ // or [MAX,MAX].
+ if (vrp_val_is_min (min))
+ min = max = vrp_val_max (type);
+ else
+ min = max = vrp_val_min (type);
+ set (min, max);
+ }
+ else
+ {
+ // The only alternative is [MIN,MAX], which is the empty range.
+ set_undefined ();
+ }
+ if (flag_checking)
+ verify_range ();
+}
+
void
irange::irange_set_anti_range (tree min, tree max)
{
gcc_checking_assert (!POLY_INT_CST_P (min));
gcc_checking_assert (!POLY_INT_CST_P (max));
+ if (TYPE_PRECISION (TREE_TYPE (min)) == 1)
+ {
+ irange_set_1bit_anti_range (min, max);
+ return;
+ }
+
// set an anti-range
tree type = TREE_TYPE (min);
signop sign = TYPE_SIGN (type);
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 8a4d8ec4207..bfc54a2473f 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -127,6 +127,8 @@ protected:
void copy_legacy_to_multi_range (const irange &);
private:
+ void irange_set_1bit_anti_range (tree, tree);
+
unsigned char m_num_ranges;
unsigned char m_max_ranges;
ENUM_BITFIELD(value_range_kind) m_kind : 8;
--
2.30.2
