| Message ID | 9ae81606-7e34-423c-b9d5-98147a43d5b6@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [COMMITTED,1/4] - Cleanup pointer_plus_operator. | expand |
Hello, Le 24/10/2024 à 14:53, Andrew MacLeod a écrit : > diff --git a/gcc/range-op-ptr.cc b/gcc/range-op-ptr.cc > index dd312a80366..ef2b2cce516 100644 > --- a/gcc/range-op-ptr.cc > +++ b/gcc/range-op-ptr.cc (...) > -void > -pointer_or_operator::wi_fold (irange &r, tree type, > - const wide_int &lh_lb, > - const wide_int &lh_ub, > - const wide_int &rh_lb, > - const wide_int &rh_ub) const > +operator_bitwise_or::fold_range (prange &r, tree type, > + const prange &op1, > + const prange &op2, > + relation_trio) const > { > // For pointer types, we are really only interested in asserting > // whether the expression evaluates to non-NULL. > - if (!wi_includes_zero_p (type, lh_lb, lh_ub) > - && !wi_includes_zero_p (type, rh_lb, rh_ub)) > + if (!op1.zero_p () || !op2.zero_p ()) > r.set_nonzero (type); this doesn't feel right. It checks that the operand range is anything but singleton [0], instead of checking that it does not contain 0. > - else if (wi_zero_p (type, lh_lb, lh_ub) && wi_zero_p (type, rh_lb, rh_ub)) > + else if (op1.zero_p () && op2.zero_p ()) > r.set_zero (type); > else > r.set_varying (type); And it makes this else branch dead. > + > + update_known_bitmask (r, BIT_IOR_EXPR, op1, op2); > + return true; > } > For example, the change makes VARYING | 0 fold to non-zero (instead of VARYING).
On 10/26/24 15:08, Mikael Morin wrote: > Hello, > > Le 24/10/2024 à 14:53, Andrew MacLeod a écrit : >> diff --git a/gcc/range-op-ptr.cc b/gcc/range-op-ptr.cc >> index dd312a80366..ef2b2cce516 100644 >> --- a/gcc/range-op-ptr.cc >> +++ b/gcc/range-op-ptr.cc > (...) >> -void >> -pointer_or_operator::wi_fold (irange &r, tree type, >> - const wide_int &lh_lb, >> - const wide_int &lh_ub, >> - const wide_int &rh_lb, >> - const wide_int &rh_ub) const >> +operator_bitwise_or::fold_range (prange &r, tree type, >> + const prange &op1, >> + const prange &op2, >> + relation_trio) const >> { >> // For pointer types, we are really only interested in asserting >> // whether the expression evaluates to non-NULL. >> - if (!wi_includes_zero_p (type, lh_lb, lh_ub) >> - && !wi_includes_zero_p (type, rh_lb, rh_ub)) >> + if (!op1.zero_p () || !op2.zero_p ()) >> r.set_nonzero (type); > > this doesn't feel right. It checks that the operand range is anything > but singleton [0], instead of checking that it does not contain 0. Hmm, i think you are right. But I also think the original is wrong? Shouldn't it be nonzero if either operand doesn't contain zero? It's Monday morning.. Check my logic please... :-) Andrew diff --git a/gcc/range-op-ptr.cc b/gcc/range-op-ptr.cc index 9f500c52d2c..ccce05035b9 100644 --- a/gcc/range-op-ptr.cc +++ b/gcc/range-op-ptr.cc @@ -388,7 +388,7 @@ operator_bitwise_or::fold_range (prange &r, tree type, { // For pointer types, we are really only interested in asserting // whether the expression evaluates to non-NULL. - if (!op1.zero_p () || !op2.zero_p ()) + if (!range_includes_zero_p (op1) || !range_includes_zero_p (op2)) r.set_nonzero (type); else if (op1.zero_p () && op2.zero_p ()) r.set_zero (type); Andrew
Le 28/10/2024 à 14:38, Andrew MacLeod a écrit : > > On 10/26/24 15:08, Mikael Morin wrote: >> Hello, >> >> Le 24/10/2024 à 14:53, Andrew MacLeod a écrit : >>> diff --git a/gcc/range-op-ptr.cc b/gcc/range-op-ptr.cc >>> index dd312a80366..ef2b2cce516 100644 >>> --- a/gcc/range-op-ptr.cc >>> +++ b/gcc/range-op-ptr.cc >> (...) >>> -void >>> -pointer_or_operator::wi_fold (irange &r, tree type, >>> - const wide_int &lh_lb, >>> - const wide_int &lh_ub, >>> - const wide_int &rh_lb, >>> - const wide_int &rh_ub) const >>> +operator_bitwise_or::fold_range (prange &r, tree type, >>> + const prange &op1, >>> + const prange &op2, >>> + relation_trio) const >>> { >>> // For pointer types, we are really only interested in asserting >>> // whether the expression evaluates to non-NULL. >>> - if (!wi_includes_zero_p (type, lh_lb, lh_ub) >>> - && !wi_includes_zero_p (type, rh_lb, rh_ub)) >>> + if (!op1.zero_p () || !op2.zero_p ()) >>> r.set_nonzero (type); >> >> this doesn't feel right. It checks that the operand range is anything >> but singleton [0], instead of checking that it does not contain 0. > > > Hmm, i think you are right. But I also think the original is wrong? > Shouldn't it be nonzero if either operand doesn't contain zero? > > It's Monday morning.. Check my logic please... :-) > The original wasn't strictly speaking wrong; I mean it was just producing more VARYING than necessary. But otherwise, yes, your logic appears correct. > > diff --git a/gcc/range-op-ptr.cc b/gcc/range-op-ptr.cc > index 9f500c52d2c..ccce05035b9 100644 > --- a/gcc/range-op-ptr.cc > +++ b/gcc/range-op-ptr.cc > @@ -388,7 +388,7 @@ operator_bitwise_or::fold_range (prange &r, tree type, > { > // For pointer types, we are really only interested in asserting > // whether the expression evaluates to non-NULL. > - if (!op1.zero_p () || !op2.zero_p ()) > + if (!range_includes_zero_p (op1) || !range_includes_zero_p (op2)) > r.set_nonzero (type); > else if (op1.zero_p () && op2.zero_p ()) > r.set_zero (type); > And this looks good.
From 57f720af635e47d8b5515eddaef9df02769f353b Mon Sep 17 00:00:00 2001 From: Andrew MacLeod <amacleod@redhat.com> Date: Wed, 23 Oct 2024 10:59:13 -0400 Subject: [PATCH 4/4] Implement pointer_or_operator. The class pointer_or is no longer used, and can be removed. Its functionality was never moved to the new dispatch system. This implements operator_bitwise_or::fold_range() for prange operands. * range-op-mixed.h (operator_bitwise_or::fold_range): Add prange variant. * range-op-ptr.cc (class pointer_or_operator): Remove. (pointer_or_operator::op1_range): Remove. (pointer_or_operator::op2_range): Remove. (pointer_or_operator::wi_fold): Remove. (operator_bitwise_or::fold_range): New prange variant. --- gcc/range-op-mixed.h | 6 +++++ gcc/range-op-ptr.cc | 63 +++++++------------------------------------- 2 files changed, 16 insertions(+), 53 deletions(-) diff --git a/gcc/range-op-mixed.h b/gcc/range-op-mixed.h index cc1db2f6775..f7c447d935e 100644 --- a/gcc/range-op-mixed.h +++ b/gcc/range-op-mixed.h @@ -809,9 +809,15 @@ protected: class operator_bitwise_or : public range_operator { public: + using range_operator::fold_range; using range_operator::op1_range; using range_operator::op2_range; using range_operator::update_bitmask; + + bool fold_range (prange &r, tree type, + const prange &op1, + const prange &op2, + relation_trio) const final override; bool op1_range (irange &r, tree type, const irange &lhs, const irange &op2, relation_trio rel = TRIO_VARYING) const override; diff --git a/gcc/range-op-ptr.cc b/gcc/range-op-ptr.cc index dd312a80366..ef2b2cce516 100644 --- a/gcc/range-op-ptr.cc +++ b/gcc/range-op-ptr.cc @@ -379,69 +379,26 @@ pointer_plus_operator::op2_range (irange &r, tree type, return true; } - -class pointer_or_operator : public range_operator -{ -public: - using range_operator::op1_range; - using range_operator::op2_range; - virtual bool op1_range (irange &r, tree type, - const irange &lhs, - const irange &op2, - relation_trio rel = TRIO_VARYING) const; - virtual bool op2_range (irange &r, tree type, - const irange &lhs, - const irange &op1, - relation_trio rel = TRIO_VARYING) const; - virtual void wi_fold (irange &r, tree type, - const wide_int &lh_lb, const wide_int &lh_ub, - const wide_int &rh_lb, const wide_int &rh_ub) const; -} op_pointer_or; - -bool -pointer_or_operator::op1_range (irange &r, tree type, - const irange &lhs, - const irange &op2 ATTRIBUTE_UNUSED, - relation_trio) const -{ - if (lhs.undefined_p ()) - return false; - if (lhs.zero_p ()) - { - r.set_zero (type); - return true; - } - r.set_varying (type); - return true; -} - bool -pointer_or_operator::op2_range (irange &r, tree type, - const irange &lhs, - const irange &op1, - relation_trio) const -{ - return pointer_or_operator::op1_range (r, type, lhs, op1); -} - -void -pointer_or_operator::wi_fold (irange &r, tree type, - const wide_int &lh_lb, - const wide_int &lh_ub, - const wide_int &rh_lb, - const wide_int &rh_ub) const +operator_bitwise_or::fold_range (prange &r, tree type, + const prange &op1, + const prange &op2, + relation_trio) const { // For pointer types, we are really only interested in asserting // whether the expression evaluates to non-NULL. - if (!wi_includes_zero_p (type, lh_lb, lh_ub) - && !wi_includes_zero_p (type, rh_lb, rh_ub)) + if (!op1.zero_p () || !op2.zero_p ()) r.set_nonzero (type); - else if (wi_zero_p (type, lh_lb, lh_ub) && wi_zero_p (type, rh_lb, rh_ub)) + else if (op1.zero_p () && op2.zero_p ()) r.set_zero (type); else r.set_varying (type); + + update_known_bitmask (r, BIT_IOR_EXPR, op1, op2); + return true; } + class operator_pointer_diff : public range_operator { using range_operator::fold_range; -- 2.45.0