One of the difficulties I ran into when familiarizing myself with
value-range.{h,cc} is that the comments and classes refer to
representations of "ranges", but the implementation has grown beyond
mere ranges of values (such as with bitmasks and NaN-tracking).
Arguably "range" could refer to the mathematical definition: the set
of possible outputs of a function, but I find it much clearer to think
of these classes as efficient representations of subsets of possible
values of a type.
This patch updates various leading comments in a way that clarifies
the intent of these classes (for me, at least).
Andrew: did I get all the details correct, and is this OK for trunk?
(assuming it bootstraps)
Thanks
Dave
gcc/ChangeLog:
* value-range.cc (irange_bitmask::irange_bitmask): Fix typo in
comment.
* value-range.h (class vrange): Update leading comment to refer
to "subsets" rather than "ranges". Allude to the available
subclasses. Warn that the classes can be over-approximations and
thus can introduce imprecision.
(class irange_bitmask): Updating leading comment to refer to
knowledge about a "value", rather than a "range". Reword
description of MASK and VALUE to clarify implementation, and
add an example.
(class irange): Update leading comment to refer to a
"subset of possible values" rather than a "range", and
that subclasses have responsibility for storage.
(class nan_state): Rewrite leading comment.
(class frange final): Update leading comment to refer to
subsets of possible values, rather than ranges, and to
consistently use "Nan" when capitalizing.
Signed-off-by: David Malcolm <[email protected]>
---
gcc/value-range.cc | 2 +-
gcc/value-range.h | 40 +++++++++++++++++++++++++++++-----------
2 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 9bd9dc7506b1..eeb3abffe6cf 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -32,7 +32,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-range.h"
// Return the bitmask inherent in a range : TYPE [MIN, MAX].
-// This use to be get_bitmask_from_range ().
+// This used to be get_bitmask_from_range ().
irange_bitmask::irange_bitmask (tree type,
const wide_int &min, const wide_int &max)
diff --git a/gcc/value-range.h b/gcc/value-range.h
index d0d1806519ae..cb8e7fe2f267 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -55,7 +55,17 @@ enum value_range_discriminator
VR_UNKNOWN
};
-// Abstract class for ranges of any of the supported types.
+// Abstract class for representing subsets of values for various
+// supported types, such as the possible values of a variable.
+//
+// There are subclasses for each of integer, floating point, and pointer
+// types, each with their own strategies for efficiently representing
+// subsets of values.
+//
+// For efficiency, we can't precisely represent any arbitrary subset
+// of values of a type (which could require 2^N bits for a type of size N)
+// Hence operations on the subclasses may introduce imprecision
+// due to over-approximating the possible subsets.
//
// To query what types ranger and the entire ecosystem can support,
// use value_range::supports_type_p(tree type). This is a static
@@ -123,13 +133,18 @@ namespace inchash
extern void add_vrange (const vrange &, hash &, unsigned flags = 0);
}
-// A pair of values representing the known bits in a range. Zero bits
+// A pair of values representing the known bits of a value. Zero bits
// in MASK cover constant values. Set bits in MASK cover unknown
-// values. VALUE are the known bits.
-//
-// Set bits in MASK (no meaningful information) must have their
-// corresponding bits in VALUE cleared, as this speeds up union and
-// intersect.
+// values. VALUE are the known bits for the bits where MASK is zero,
+// and must be zero for the unknown bits where MASK is set (needed as an
+// optimization of union and intersect)
+// For example:
+// VALUE: [..., 0, 1, 0]
+// MASK: [..., 1, 0, 0]
+// ^ ^ ^
+// | | known bit: {0}
+// | known bit: {1}
+// unknown bit: {0, 1}
class irange_bitmask
{
@@ -267,7 +282,8 @@ irange_bitmask::intersect (const irange_bitmask &src)
return true;
}
-// An integer range without any storage.
+// A subset of possible values for an integer type, leaving
+// allocation of storage to subclasses.
class irange : public vrange
{
@@ -470,7 +486,9 @@ public:
tree ubound () const final override;
};
-// The NAN state as an opaque object.
+// The possible NAN state of a floating point value as an opaque object.
+// This represents one of the four subsets of { -NaN, +NaN },
+// i.e. one of {}, { -NaN }, { +NaN}, or { -NaN, +NaN }.
class nan_state
{
@@ -521,10 +539,10 @@ nan_state::neg_p () const
return m_neg_nan;
}
-// A floating point range.
+// A subset of possible values for a floating point type.
//
// The representation is a type with a couple of endpoints, unioned
-// with the set of { -NAN, +Nan }.
+// with a subset of { -NaN, +NaN }.
class frange final : public vrange
{
--
2.26.3
