]> gcc.gnu.org Git - gcc.git/commitdiff
Only return changed=true in union_nonzero when appropriate.
authorAldy Hernandez <aldyh@redhat.com>
Mon, 15 May 2023 13:10:11 +0000 (15:10 +0200)
committerAldy Hernandez <aldyh@redhat.com>
Mon, 15 May 2023 17:03:19 +0000 (19:03 +0200)
irange::union_ was being overly pessimistic in its return value.  It
was returning false when the nonzero mask was possibly the same.

The reason for this is because the nonzero mask is not entirely kept
up to date.  We avoid setting it up when a new range is set (from a
set, intersect, union, etc), because calculating a mask from a range
is measurably expensive.  However, irange::get_nonzero_bits() will
always return the correct mask because it will calculate the nonzero
mask inherit in the mask on the fly and bitwise or it with the saved
mask.  This was an optimization because last release it was a big
penalty to keep the mask up to date.  This may not necessarily be the
case with the conversion to wide_int's.  We should investigate.

Just to be clear, the result from get_nonzero_bits() is always correct
as seen by the user, but the wide_int in the irange does not contain
all the information, since part of the nonzero bits can be determined
by the range itself, on the fly.

The fix here is to make sure that the result the user sees (callers of
get_nonzero_bits()) changed when unioning bits.  This allows
ipcp_vr_lattice::meet_with_1 to avoid unnecessary copies when
determining if a range changed.

This patch yields an 6.89% improvement to the ipa_cp pass.  I'm
including the IPA changes in this patch, as it's a testcase of sorts for
the change.

gcc/ChangeLog:

* ipa-cp.cc (ipcp_vr_lattice::meet_with_1): Avoid unnecessary
range copying
* value-range.cc (irange::union_nonzero_bits): Return TRUE only
when range changed.

gcc/ipa-cp.cc
gcc/value-range.cc

index 1f5e0e138728072a4bf28288846c7f1164c223bf..8cd0fa2cae796cbce0fb9be4121c36f49b08d698 100644 (file)
@@ -1040,9 +1040,16 @@ ipcp_vr_lattice::meet_with_1 (const value_range *other_vr)
   if (other_vr->varying_p ())
     return set_to_bottom ();
 
-  value_range save (m_vr);
-  m_vr.union_ (*other_vr);
-  return m_vr != save;
+  bool res;
+  if (flag_checking)
+    {
+      value_range save (m_vr);
+      res = m_vr.union_ (*other_vr);
+      gcc_assert (res == (m_vr != save));
+    }
+  else
+    res = m_vr.union_ (*other_vr);
+  return res;
 }
 
 /* Return true if value range information in the lattice is yet unknown.  */
index def9299dc0ed780c34c7d011a0b0ece0b2b46517..a341cece86df14d56777f3119d4c8aa4b15f4c28 100644 (file)
@@ -1859,12 +1859,13 @@ irange::union_nonzero_bits (const irange &r)
   bool changed = false;
   if (m_nonzero_mask != r.m_nonzero_mask)
     {
-      m_nonzero_mask = get_nonzero_bits () | r.get_nonzero_bits ();
+      wide_int save = get_nonzero_bits ();
+      m_nonzero_mask = save | r.get_nonzero_bits ();
       // No need to call set_range_from_nonzero_bits, because we'll
       // never narrow the range.  Besides, it would cause endless
       // recursion because of the union_ in
       // set_range_from_nonzero_bits.
-      changed = true;
+      changed = m_nonzero_mask != save;
     }
   normalize_kind ();
   if (flag_checking)
This page took 0.07638 seconds and 5 git commands to generate.