public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Aldy Hernandez <aldyh@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r15-41] Callers of irange_bitmask must normalize value/mask pairs.
Date: Sun, 28 Apr 2024 19:04:58 +0000 (GMT)	[thread overview]
Message-ID: <20240428190458.0C9283858C32@sourceware.org> (raw)

https://gcc.gnu.org/g:d71308d5a681de008888ea291136c162e5b46c7c

commit r15-41-gd71308d5a681de008888ea291136c162e5b46c7c
Author: Aldy Hernandez <aldyh@redhat.com>
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.

                 reply	other threads:[~2024-04-28 19:04 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240428190458.0C9283858C32@sourceware.org \
    --to=aldyh@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).