From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2136) id 0C9283858C32; Sun, 28 Apr 2024 19:04:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0C9283858C32 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1714331098; bh=kwpXKqzpdGdE4C6RUpFLIj4nkOawpOSQr3ByMYe9jNs=; h=From:To:Subject:Date:From; b=HUKkG30DVTdXeEMKvvEUQ1cStwnPvt9UoFXxlVWU08yfBkZ0ivznsxMNsp2VymCTO W5doVveL50yn9He6Pp6sPZSHMB3Ntr96wQaWFWhDMiM7y4AwkAOGytZ9TUEvH/e7Y1 Ua93/6CGW5KAr7ftqnxvAcOfN8ihtZv4ZFYeFqig= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Aldy Hernandez To: gcc-cvs@gcc.gnu.org Subject: [gcc r15-41] Callers of irange_bitmask must normalize value/mask pairs. X-Act-Checkin: gcc X-Git-Author: Aldy Hernandez X-Git-Refname: refs/heads/master X-Git-Oldrev: 3b9abfd2df5fe720798aab1e21b4a11876607561 X-Git-Newrev: d71308d5a681de008888ea291136c162e5b46c7c Message-Id: <20240428190458.0C9283858C32@sourceware.org> Date: Sun, 28 Apr 2024 19:04:58 +0000 (GMT) List-Id: https://gcc.gnu.org/g:d71308d5a681de008888ea291136c162e5b46c7c commit r15-41-gd71308d5a681de008888ea291136c162e5b46c7c Author: Aldy Hernandez Date: Tue Apr 23 10:12:56 2024 +0200 Callers of irange_bitmask must normalize value/mask pairs. As per the documentation, irange_bitmask must have the unknown bits in the mask set to 0 in the value field. Even though we say we must have normalized value/mask bits, we don't enforce it, opting to normalize on the fly in union and intersect. Avoiding this lazy enforcing as well as the extra saving/restoring involved in returning the changed status, gives us a performance increase of 1.25% for VRP and 1.51% for ipa-CP. gcc/ChangeLog: * tree-ssa-ccp.cc (ccp_finalize): Normalize before calling set_bitmask. * value-range.cc (irange::intersect_bitmask): Calculate changed irange_bitmask bits on our own. (irange::union_bitmask): Same. (irange_bitmask::verify_mask): Verify that bits are normalized. * value-range.h (irange_bitmask::union_): Do not normalize. Remove return value. (irange_bitmask::intersect): Same. Diff: --- gcc/tree-ssa-ccp.cc | 1 + gcc/value-range.cc | 7 +++++-- gcc/value-range.h | 24 ++++++------------------ 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc index f6a5cd0ee6e..3749126b5f7 100644 --- a/gcc/tree-ssa-ccp.cc +++ b/gcc/tree-ssa-ccp.cc @@ -1024,6 +1024,7 @@ ccp_finalize (bool nonzero_p) unsigned int precision = TYPE_PRECISION (TREE_TYPE (val->value)); wide_int value = wi::to_wide (val->value); wide_int mask = wide_int::from (val->mask, precision, UNSIGNED); + value = value & ~mask; set_bitmask (name, value, mask); } } diff --git a/gcc/value-range.cc b/gcc/value-range.cc index a27de5534e1..ca6d521c625 100644 --- a/gcc/value-range.cc +++ b/gcc/value-range.cc @@ -2067,7 +2067,8 @@ irange::intersect_bitmask (const irange &r) irange_bitmask bm = get_bitmask (); irange_bitmask save = bm; - if (!bm.intersect (r.get_bitmask ())) + bm.intersect (r.get_bitmask ()); + if (save == bm) return false; m_bitmask = bm; @@ -2099,7 +2100,8 @@ irange::union_bitmask (const irange &r) irange_bitmask bm = get_bitmask (); irange_bitmask save = bm; - if (!bm.union_ (r.get_bitmask ())) + bm.union_ (r.get_bitmask ()); + if (save == bm) return false; m_bitmask = bm; @@ -2133,6 +2135,7 @@ void irange_bitmask::verify_mask () const { gcc_assert (m_value.get_precision () == m_mask.get_precision ()); + gcc_checking_assert (wi::bit_and (m_mask, m_value) == 0); } void diff --git a/gcc/value-range.h b/gcc/value-range.h index 0ab717697f0..11c73faca1b 100644 --- a/gcc/value-range.h +++ b/gcc/value-range.h @@ -139,8 +139,8 @@ public: void set_unknown (unsigned prec); bool unknown_p () const; unsigned get_precision () const; - bool union_ (const irange_bitmask &src); - bool intersect (const irange_bitmask &src); + void union_ (const irange_bitmask &src); + void intersect (const irange_bitmask &src); bool operator== (const irange_bitmask &src) const; bool operator!= (const irange_bitmask &src) const { return !(*this == src); } void verify_mask () const; @@ -233,29 +233,18 @@ irange_bitmask::operator== (const irange_bitmask &src) const return m_value == src.m_value && m_mask == src.m_mask; } -inline bool -irange_bitmask::union_ (const irange_bitmask &orig_src) +inline void +irange_bitmask::union_ (const irange_bitmask &src) { - // Normalize mask. - irange_bitmask src (orig_src.m_value & ~orig_src.m_mask, orig_src.m_mask); - m_value &= ~m_mask; - - irange_bitmask save (*this); m_mask = (m_mask | src.m_mask) | (m_value ^ src.m_value); m_value = m_value & src.m_value; if (flag_checking) verify_mask (); - return *this != save; } -inline bool -irange_bitmask::intersect (const irange_bitmask &orig_src) +inline void +irange_bitmask::intersect (const irange_bitmask &src) { - // Normalize mask. - irange_bitmask src (orig_src.m_value & ~orig_src.m_mask, orig_src.m_mask); - m_value &= ~m_mask; - - irange_bitmask save (*this); // If we have two known bits that are incompatible, the resulting // bit is undefined. It is unclear whether we should set the entire // range to UNDEFINED, or just a subset of it. For now, set the @@ -274,7 +263,6 @@ irange_bitmask::intersect (const irange_bitmask &orig_src) } if (flag_checking) verify_mask (); - return *this != save; } // An integer range without any storage.