diff mbox series

TYPE_{MIN/MAX}_VALUE for floats?

Message ID CAGm3qMXYATzMsLq2-YSHfA+pFTrM376Fn=E3iQ=Z4N3FRu-EPA@mail.gmail.com
State Accepted, archived
Headers
Series TYPE_{MIN/MAX}_VALUE for floats? |

Checks

Context Check Description
snail/gcc-patches-check success Github commit url

Commit Message

Aldy Hernandez Sept. 22, 2022, 3:02 p.m. UTC 
  It has always irritated me that we don't have TYPE_MIN_VALUE and
TYPE_MAX_VALUE for floats (and for pointers for that matter).  This
means, we have to recalculate it ad-nauseum in vrp_val_min and
vrp_val_max.

I know we have dconstinf and dconstninf for floats, which we can just
wrap around a TREE_REAL_CST, but it still seems like we should be more
consistent here.  If we know the endpoint for a type, we should cache
it in it.

Furthermore, just the way we're chopping off NANs in the frange::set()
routine, we should be able to chop off things outside the min/max
representable range, at least for -ffinite-math-only.  For example,
the endpoints to VR_VARYING for a float in -ffinite-math-only should
be real_{min/max}_representable(), which REAL_VALUE_TYPE already
provides.   I am testing a patch to do this, but am unhappy that we
have recalculate things.

Is there a reason we can't store these in the type?

I tried the naive attached approach, but I quickly ran into LTO woes:

FAIL: gcc.c-torture/execute/ieee/20001122-1.c compilation,  -O2 -flto
-fno-use-linker-plugin -flto-partition=none
 (internal compiler error: 'verify_type' failed)

$ ./xgcc -B./ a.c -O2 -flto -w
lto1: error: type variant differs by TYPE_MAX_VALUE

So I clearly don't know what I'm doing.

Would folks be ok with filling TYPE_MIN_VALUE and friends for floats,
and if so, could someone give me a hand here?  What am I missing?

Thanks.
Aldy

p.s. Now that we're onto this subject, in the distant future, I'd
actually like to store a vrange in the tree type.  I mean, they are
first class citizens in the SSA name now, and we have a typeless way
of storing ranges in GC space.  Anywho, that's for the future, cause I
like the pain... just wanted to gauge the temperature on that one as
well.

Comments

Jakub Jelinek Sept. 22, 2022, 3:22 p.m. UTC | #1 
On Thu, Sep 22, 2022 at 05:02:19PM +0200, Aldy Hernandez wrote:
> It has always irritated me that we don't have TYPE_MIN_VALUE and
> TYPE_MAX_VALUE for floats (and for pointers for that matter).  This
> means, we have to recalculate it ad-nauseum in vrp_val_min and
> vrp_val_max.
> 
> I know we have dconstinf and dconstninf for floats, which we can just
> wrap around a TREE_REAL_CST, but it still seems like we should be more
> consistent here.  If we know the endpoint for a type, we should cache
> it in it.

This looks problematic.
While for !MODE_HAS_INFINITIES there are clear values, otherwise
the flag_finite_math_only flag has Optimization keyword, so it can change
between different functions, while a type is a global entity that can be
used by both __attribute__((optimize ("Ofast"))) and standard floating point
functions.
In some sense it is similar to TYPE_MODE which for vectors needs to be
actually a function call that decides based on the current function.
But then, having it in TYPE_*_VALUE doesn't have the benefits you want from
it...

	Jakub
Aldy Hernandez Sept. 22, 2022, 3:45 p.m. UTC | #2 
On Thu, Sep 22, 2022 at 5:22 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Sep 22, 2022 at 05:02:19PM +0200, Aldy Hernandez wrote:
> > It has always irritated me that we don't have TYPE_MIN_VALUE and
> > TYPE_MAX_VALUE for floats (and for pointers for that matter).  This
> > means, we have to recalculate it ad-nauseum in vrp_val_min and
> > vrp_val_max.
> >
> > I know we have dconstinf and dconstninf for floats, which we can just
> > wrap around a TREE_REAL_CST, but it still seems like we should be more
> > consistent here.  If we know the endpoint for a type, we should cache
> > it in it.
>
> This looks problematic.
> While for !MODE_HAS_INFINITIES there are clear values, otherwise
> the flag_finite_math_only flag has Optimization keyword, so it can change
> between different functions, while a type is a global entity that can be
> used by both __attribute__((optimize ("Ofast"))) and standard floating point
> functions.

Oh...it can have different values in different functions?  Yeah,
that's not gonna work.  Oh well, thanks.

Aldy
diff mbox series

Patch

diff --git a/gcc/stor-layout.cc b/gcc/stor-layout.cc
index 88923c4136b..98f268d9f5a 100644
--- a/gcc/stor-layout.cc
+++ b/gcc/stor-layout.cc
@@ -43,6 +43,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "attribs.h"
 #include "debug.h"
 #include "calls.h"
+#include "real.h"
 
 /* Data type for the expressions representing sizes of data types.
    It is the first integer type laid out.  */
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 4165cbd7c3b..7a1fc6c4888 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -7620,6 +7620,31 @@  build_offset_type (tree basetype, tree type)
   return t;
 }
 
+/* Create a floating point type with PRECISION.  */
+
+tree
+build_float_type (unsigned precision)
+{
+  tree type = make_node (REAL_TYPE);
+  TYPE_PRECISION (type) = precision;
+  layout_type (type);
+
+  if (flag_finite_math_only)
+    {
+      REAL_VALUE_TYPE min, max;
+      real_min_representable (&min, type);
+      real_max_representable (&max, type);
+      TYPE_MIN_VALUE (type) = build_real (type, min);
+      TYPE_MAX_VALUE (type) = build_real (type, max);
+    }
+  else
+    {
+      TYPE_MIN_VALUE (type) = build_real (type, dconstninf);
+      TYPE_MAX_VALUE (type) = build_real (type, dconstinf);
+    }
+  return type;
+}
+
 /* Create a complex type whose components are COMPONENT_TYPE.
 
    If NAMED is true, the type is given a TYPE_NAME.  We do not always
@@ -9427,17 +9452,9 @@  build_common_tree_nodes (bool signed_char)
 
   pointer_sized_int_node = build_nonstandard_integer_type (POINTER_SIZE, 1);
 
-  float_type_node = make_node (REAL_TYPE);
-  TYPE_PRECISION (float_type_node) = FLOAT_TYPE_SIZE;
-  layout_type (float_type_node);
-
-  double_type_node = make_node (REAL_TYPE);
-  TYPE_PRECISION (double_type_node) = DOUBLE_TYPE_SIZE;
-  layout_type (double_type_node);
-
-  long_double_type_node = make_node (REAL_TYPE);
-  TYPE_PRECISION (long_double_type_node) = LONG_DOUBLE_TYPE_SIZE;
-  layout_type (long_double_type_node);
+  float_type_node = build_float_type (FLOAT_TYPE_SIZE);
+  double_type_node = build_float_type (DOUBLE_TYPE_SIZE);
+  long_double_type_node = build_float_type (LONG_DOUBLE_TYPE_SIZE);
 
   for (i = 0; i < NUM_FLOATN_NX_TYPES; i++)
     {
diff --git a/gcc/tree.h b/gcc/tree.h
index 266e24a0563..b83fac17f1a 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4729,6 +4729,7 @@  extern tree build_varargs_function_type_array (tree, int, tree *);
 extern tree build_method_type_directly (tree, tree, tree);
 extern tree build_method_type (tree, tree);
 extern tree build_offset_type (tree, tree);
+extern tree build_float_type (unsigned);
 extern tree build_complex_type (tree, bool named = false);
 extern tree array_type_nelts (const_tree);