From 6a7354d3494665d46f8cbfc71c58f784c02142ff Mon Sep 17 00:00:00 2001 From: Aldy Hernandez Date: Mon, 15 May 2023 15:10:11 +0200 Subject: [PATCH] Only return changed=true in union_nonzero when appropriate. 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 | 13 ++++++++++--- gcc/value-range.cc | 5 +++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc index 1f5e0e13872..8cd0fa2cae7 100644 --- a/gcc/ipa-cp.cc +++ b/gcc/ipa-cp.cc @@ -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. */ diff --git a/gcc/value-range.cc b/gcc/value-range.cc index def9299dc0e..a341cece86d 100644 --- a/gcc/value-range.cc +++ b/gcc/value-range.cc @@ -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) -- 2.40.0