[RFC] Add op1_range for __builtin_signbit.
Checks
Commit Message
This is the op1_range range-op entry for __builtin_signbit. It allows
us to wind back through a call to signbit.
For example, on the true side of if (__builtin_signbit(x_5) != 0) we
can crop down the range of x_5 to:
[frange] float [-Inf, -0.0 (-0x0.0p+0)] -NAN
Similarly on the false side, we can crop to:
[frange] float [0.0 (0x0.0p+0), +Inf] +NAN
This brings about an interesting question, can we fold the second
conditional here as always false?
void foo(float x)
{
if (__builtin_signbit (x))
{
if (x > 0.0)
link_error();
}
}
The only values for x at the second conditional are negative values,
or -NAN, so it will always evaluate to false. ISTM that we
*shouldn't* fold this conditional as there is user observable behavior
if there is a signaling NAN. For that matter, that is exactly what we
do in ranger-ops. We leave the conditional in place, but ranger
is able to determine that "x" is UNDEFINED on the path leading up to
link_error:
=========== BB 3 ============
Imports: x_3(D)
Exports: x_3(D)
x_3(D) [frange] float [-Inf, -0.0 (-0x0.0p+0)] -NAN
<bb 3> :
if (x_3(D) > 0.0)
goto <bb 4>; [INV]
else
goto <bb 5>; [INV]
3->4 (T) x_3(D) : [frange] UNDEFINED
3->5 (F) x_3(D) : [frange] float [-Inf, -0.0 (-0x0.0p+0)] -NAN
This would allow users of ranger to query x_3 on the 3->4 and notice
it's unreachable, without VRP removing the conditional.
I believe this is correct, but would like confirmation from the experts.
gcc/ChangeLog:
* gimple-range-op.cc: Add op1_range entry for __builtin_signbit.
gcc/testsuite/ChangeLog:
* gcc.dg/tree-ssa/vrp-float-signbit-3.c: New test.
---
gcc/gimple-range-op.cc | 20 +++++++++++++++++++
.../gcc.dg/tree-ssa/vrp-float-signbit-3.c | 14 +++++++++++++
2 files changed, 34 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-3.c
Comments
I have committed this patch, as the __builtin_signbit range-op entry
is correct. I'd still like clarification on the folding).
Aldy
On Thu, Oct 6, 2022 at 12:51 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> This is the op1_range range-op entry for __builtin_signbit. It allows
> us to wind back through a call to signbit.
>
> For example, on the true side of if (__builtin_signbit(x_5) != 0) we
> can crop down the range of x_5 to:
>
> [frange] float [-Inf, -0.0 (-0x0.0p+0)] -NAN
>
> Similarly on the false side, we can crop to:
>
> [frange] float [0.0 (0x0.0p+0), +Inf] +NAN
>
> This brings about an interesting question, can we fold the second
> conditional here as always false?
>
> void foo(float x)
> {
> if (__builtin_signbit (x))
> {
> if (x > 0.0)
> link_error();
> }
> }
>
> The only values for x at the second conditional are negative values,
> or -NAN, so it will always evaluate to false. ISTM that we
> *shouldn't* fold this conditional as there is user observable behavior
> if there is a signaling NAN. For that matter, that is exactly what we
> do in ranger-ops. We leave the conditional in place, but ranger
> is able to determine that "x" is UNDEFINED on the path leading up to
> link_error:
>
> =========== BB 3 ============
> Imports: x_3(D)
> Exports: x_3(D)
> x_3(D) [frange] float [-Inf, -0.0 (-0x0.0p+0)] -NAN
> <bb 3> :
> if (x_3(D) > 0.0)
> goto <bb 4>; [INV]
> else
> goto <bb 5>; [INV]
>
> 3->4 (T) x_3(D) : [frange] UNDEFINED
> 3->5 (F) x_3(D) : [frange] float [-Inf, -0.0 (-0x0.0p+0)] -NAN
>
> This would allow users of ranger to query x_3 on the 3->4 and notice
> it's unreachable, without VRP removing the conditional.
>
> I believe this is correct, but would like confirmation from the experts.
>
> gcc/ChangeLog:
>
> * gimple-range-op.cc: Add op1_range entry for __builtin_signbit.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/vrp-float-signbit-3.c: New test.
> ---
> gcc/gimple-range-op.cc | 20 +++++++++++++++++++
> .../gcc.dg/tree-ssa/vrp-float-signbit-3.c | 14 +++++++++++++
> 2 files changed, 34 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-3.c
>
> diff --git a/gcc/gimple-range-op.cc b/gcc/gimple-range-op.cc
> index 42ebc7d6ce5..abc33e7af0c 100644
> --- a/gcc/gimple-range-op.cc
> +++ b/gcc/gimple-range-op.cc
> @@ -306,6 +306,7 @@ class cfn_signbit : public range_operator_float
> {
> public:
> using range_operator_float::fold_range;
> + using range_operator_float::op1_range;
> virtual bool fold_range (irange &r, tree type, const frange &lh,
> const irange &, relation_kind) const
> {
> @@ -320,6 +321,25 @@ public:
> }
> return false;
> }
> + virtual bool op1_range (frange &r, tree type, const irange &lhs,
> + const frange &, relation_kind) const override
> + {
> + if (lhs.zero_p ())
> + {
> + r.set (type, dconst0, frange_val_max (type));
> + r.update_nan (false);
> + return true;
> + }
> + if (!lhs.contains_p (build_zero_cst (lhs.type ())))
> + {
> + REAL_VALUE_TYPE dconstm0 = dconst0;
> + dconstm0.sign = 1;
> + r.set (type, frange_val_min (type), dconstm0);
> + r.update_nan (true);
> + return true;
> + }
> + return false;
> + }
> } op_cfn_signbit;
>
> // Implement range operator for CFN_BUILT_IN_TOUPPER and CFN_BUILT_IN_TOLOWER.
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-3.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-3.c
> new file mode 100644
> index 00000000000..dd3090aeb10
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-3.c
> @@ -0,0 +1,14 @@
> +// { dg-do compile }
> +// { dg-options "-O2 -ffast-math -fdump-tree-evrp" }
> +
> +void link_error();
> +
> +void foo(float x)
> +{
> + if (__builtin_signbit (x))
> + {
> + if (x > 0.0)
> + link_error();
> + }
> +}
> +// { dg-final { scan-tree-dump-not "link_error" "evrp" } }
> --
> 2.37.1
>
@@ -306,6 +306,7 @@ class cfn_signbit : public range_operator_float
{
public:
using range_operator_float::fold_range;
+ using range_operator_float::op1_range;
virtual bool fold_range (irange &r, tree type, const frange &lh,
const irange &, relation_kind) const
{
@@ -320,6 +321,25 @@ public:
}
return false;
}
+ virtual bool op1_range (frange &r, tree type, const irange &lhs,
+ const frange &, relation_kind) const override
+ {
+ if (lhs.zero_p ())
+ {
+ r.set (type, dconst0, frange_val_max (type));
+ r.update_nan (false);
+ return true;
+ }
+ if (!lhs.contains_p (build_zero_cst (lhs.type ())))
+ {
+ REAL_VALUE_TYPE dconstm0 = dconst0;
+ dconstm0.sign = 1;
+ r.set (type, frange_val_min (type), dconstm0);
+ r.update_nan (true);
+ return true;
+ }
+ return false;
+ }
} op_cfn_signbit;
// Implement range operator for CFN_BUILT_IN_TOUPPER and CFN_BUILT_IN_TOLOWER.
new file mode 100644
@@ -0,0 +1,14 @@
+// { dg-do compile }
+// { dg-options "-O2 -ffast-math -fdump-tree-evrp" }
+
+void link_error();
+
+void foo(float x)
+{
+ if (__builtin_signbit (x))
+ {
+ if (x > 0.0)
+ link_error();
+ }
+}
+// { dg-final { scan-tree-dump-not "link_error" "evrp" } }