diff mbox series

ipa: Compare jump functions in ICF (PR 113907)

Message ID ri6le5s25kl.fsf@virgil.suse.cz
State New
Headers show
Series ipa: Compare jump functions in ICF (PR 113907) | expand

Commit Message

Martin Jambor April 4, 2024, 9:14 p.m. UTC
Hello,

In PR 113907 comment #58, Honza found a case where ICF thinks bodies
of functions are equivalent but becaise of difference in aliases in a
memory access, different aggregate jump functions are associated with
supposedly equivalent call statements.  This patch adds a way to
compare jump functions and plugs it into ICF to avoid the issue.

The patch has been approved by Honza in Bugzilla.  Together with the
patch for PR 113359, it has passed bootstrap, LTO bootstrap and LTO
profiledbootstrap and testing on x86_64-linux and bootstrap and LTO
bootstrap on ppc64le-linux.  It also passed normal bootstrap on
aarch64-linux but there many testcases failed because the compiler timed
out.  The machine is old and slow and might have been oversubscribed so
my plan is to try again on gcc185 from cfarm.  If that goes well, I
intend to commit the patch and then start working on backports.

Martin


gcc/ChangeLog:

2024-03-20  Martin Jambor  <mjambor@suse.cz>

	PR ipa/113907
	* ipa-prop.h (class ipa_vr): Declare new overload of a member function
	equal_p.
	(ipa_jump_functions_equivalent_p): Declare.
	* ipa-prop.cc (ipa_vr::equal_p): New function.
	(ipa_agg_pass_through_jf_equivalent_p): Likewise.
	(ipa_agg_jump_functions_equivalent_p): Likewise.
	(ipa_jump_functions_equivalent_p): Likewise.
	* ipa-cp.h (values_equal_for_ipcp_p): Declare.
	* ipa-cp.cc (values_equal_for_ipcp_p): Make function public.
	* ipa-icf-gimple.cc: Include alloc-pool.h, symbol-summary.h, sreal.h,
	ipa-cp.h and ipa-prop.h.
	(func_checker::compare_gimple_call): Comapre jump functions.

gcc/testsuite/ChangeLog:

2024-03-20  Martin Jambor  <mjambor@suse.cz>

	PR ipa/113907
	* gcc.dg/lto/pr113907_0.c: New.
	* gcc.dg/lto/pr113907_1.c: Likewise.
	* gcc.dg/lto/pr113907_2.c: Likewise.
---
 gcc/ipa-cp.cc                         |   2 +-
 gcc/ipa-cp.h                          |   2 +
 gcc/ipa-icf-gimple.cc                 |  30 +++++
 gcc/ipa-prop.cc                       | 167 ++++++++++++++++++++++++++
 gcc/ipa-prop.h                        |   3 +
 gcc/testsuite/gcc.dg/lto/pr113907_0.c |  18 +++
 gcc/testsuite/gcc.dg/lto/pr113907_1.c |  35 ++++++
 gcc/testsuite/gcc.dg/lto/pr113907_2.c |  11 ++
 8 files changed, 267 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/lto/pr113907_0.c
 create mode 100644 gcc/testsuite/gcc.dg/lto/pr113907_1.c
 create mode 100644 gcc/testsuite/gcc.dg/lto/pr113907_2.c
diff mbox series

Patch

diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
index 2a1da631e9c..b7add455bd5 100644
--- a/gcc/ipa-cp.cc
+++ b/gcc/ipa-cp.cc
@@ -201,7 +201,7 @@  ipcp_lattice<valtype>::is_single_const ()
 
 /* Return true iff X and Y should be considered equal values by IPA-CP.  */
 
-static bool
+bool
 values_equal_for_ipcp_p (tree x, tree y)
 {
   gcc_checking_assert (x != NULL_TREE && y != NULL_TREE);
diff --git a/gcc/ipa-cp.h b/gcc/ipa-cp.h
index 0b3cfe4b526..7ff74fb5c98 100644
--- a/gcc/ipa-cp.h
+++ b/gcc/ipa-cp.h
@@ -289,4 +289,6 @@  public:
   bool virt_call = false;
 };
 
+bool values_equal_for_ipcp_p (tree x, tree y);
+
 #endif /* IPA_CP_H */
diff --git a/gcc/ipa-icf-gimple.cc b/gcc/ipa-icf-gimple.cc
index 8c2df7a354e..17f62bec068 100644
--- a/gcc/ipa-icf-gimple.cc
+++ b/gcc/ipa-icf-gimple.cc
@@ -41,7 +41,12 @@  along with GCC; see the file COPYING3.  If not see
 #include "gimple-walk.h"
 
 #include "tree-ssa-alias-compare.h"
+#include "alloc-pool.h"
+#include "symbol-summary.h"
 #include "ipa-icf-gimple.h"
+#include "sreal.h"
+#include "ipa-cp.h"
+#include "ipa-prop.h"
 
 namespace ipa_icf_gimple {
 
@@ -714,6 +719,31 @@  func_checker::compare_gimple_call (gcall *s1, gcall *s2)
       && !compatible_types_p (TREE_TYPE (t1), TREE_TYPE (t2)))
     return return_false_with_msg ("GIMPLE internal call LHS type mismatch");
 
+  if (!gimple_call_internal_p (s1))
+    {
+      cgraph_edge *e1 = cgraph_node::get (m_source_func_decl)->get_edge (s1);
+      cgraph_edge *e2 = cgraph_node::get (m_target_func_decl)->get_edge (s2);
+      class ipa_edge_args *args1 = ipa_edge_args_sum->get (e1);
+      class ipa_edge_args *args2 = ipa_edge_args_sum->get (e2);
+      if ((args1 != nullptr) != (args2 != nullptr))
+	return return_false_with_msg ("ipa_edge_args mismatch");
+      if (args1)
+	{
+	  int n1 = ipa_get_cs_argument_count (args1);
+	  int n2 = ipa_get_cs_argument_count (args2);
+	  if (n1 != n2)
+	    return return_false_with_msg ("ipa_edge_args nargs mismatch");
+	  for (int i = 0; i < n1; i++)
+	    {
+	      struct ipa_jump_func *jf1 = ipa_get_ith_jump_func (args1, i);
+	      struct ipa_jump_func *jf2 = ipa_get_ith_jump_func (args2, i);
+	      if (((jf1 != nullptr) != (jf2 != nullptr))
+		  || (jf1 && !ipa_jump_functions_equivalent_p (jf1, jf2)))
+		return return_false_with_msg ("jump function mismatch");
+	    }
+	}
+    }
+
   return compare_operand (t1, t2, get_operand_access_type (&map, t1));
 }
 
diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
index e8e4918d5a8..374e998aa64 100644
--- a/gcc/ipa-prop.cc
+++ b/gcc/ipa-prop.cc
@@ -156,6 +156,20 @@  ipa_vr::equal_p (const vrange &r) const
   return (types_compatible_p (m_type, r.type ()) && m_storage->equal_p (r));
 }
 
+bool
+ipa_vr::equal_p (const ipa_vr &o) const
+{
+  if (!known_p ())
+    return !o.known_p ();
+
+  if (!types_compatible_p (m_type, o.m_type))
+    return false;
+
+  Value_Range r;
+  o.get_vrange (r);
+  return m_storage->equal_p (r);
+}
+
 void
 ipa_vr::get_vrange (Value_Range &r) const
 {
@@ -6057,4 +6071,157 @@  ipa_prop_cc_finalize (void)
   ipa_node_params_sum = NULL;
 }
 
+/* Return true if the two pass_through components of two jump functions are
+   known to be equivalent.  AGG_JF denotes whether they are part of aggregate
+   functions or not.  The function can be used before the IPA phase of IPA-CP
+   or inlining because it cannot cope with refdesc changes these passes can
+   carry out.  */
+
+static bool
+ipa_agg_pass_through_jf_equivalent_p (ipa_pass_through_data *ipt1,
+				      ipa_pass_through_data *ipt2,
+				      bool agg_jf)
+
+{
+  gcc_assert (agg_jf ||
+	      (!ipt1->refdesc_decremented && !ipt2->refdesc_decremented));
+  if (ipt1->operation != ipt2->operation
+      || ipt1->formal_id != ipt2->formal_id
+      || (!agg_jf && (ipt1->agg_preserved != ipt2->agg_preserved)))
+    return false;
+  if (((ipt1->operand != NULL_TREE) != (ipt2->operand != NULL_TREE))
+      || (ipt1->operand
+	  && !values_equal_for_ipcp_p (ipt1->operand, ipt2->operand)))
+    return false;
+  return true;
+}
+
+/* Return true if the two aggregate jump functions are known to be equivalent.
+   The function can be used before the IPA phase of IPA-CP or inlining because
+   it cannot cope with refdesc changes these passes can carry out.  */
+
+static bool
+ipa_agg_jump_functions_equivalent_p (ipa_agg_jf_item *ajf1,
+				     ipa_agg_jf_item *ajf2)
+{
+  if (ajf1->offset != ajf2->offset
+      || ajf1->jftype != ajf2->jftype
+      || !types_compatible_p (ajf1->type, ajf2->type))
+    return false;
+
+  switch (ajf1->jftype)
+    {
+    case IPA_JF_CONST:
+      if (!values_equal_for_ipcp_p (ajf1->value.constant,
+				    ajf2->value.constant))
+	return false;
+      break;
+    case IPA_JF_PASS_THROUGH:
+      {
+	ipa_pass_through_data *ipt1 = &ajf1->value.pass_through;
+	ipa_pass_through_data *ipt2 = &ajf2->value.pass_through;
+	if (!ipa_agg_pass_through_jf_equivalent_p (ipt1, ipt2, true))
+	  return false;
+      }
+      break;
+    case IPA_JF_LOAD_AGG:
+      {
+	ipa_load_agg_data *ila1 = &ajf1->value.load_agg;
+	ipa_load_agg_data *ila2 = &ajf2->value.load_agg;
+	if (!ipa_agg_pass_through_jf_equivalent_p (&ila1->pass_through,
+						   &ila2->pass_through, true))
+	  return false;
+	if (ila1->offset != ila2->offset
+	    || ila1->by_ref != ila2->by_ref
+	    || !types_compatible_p (ila1->type, ila2->type))
+	  return false;
+      }
+      break;
+    default:
+	gcc_unreachable ();
+    }
+  return true;
+}
+
+/* Return true if the two jump functions are known to be equivalent.  The
+   function can be used before the IPA phase of IPA-CP or inlining because it
+   cannot cope with refdesc changes these passes can carry out.  */
+
+bool
+ipa_jump_functions_equivalent_p (ipa_jump_func *jf1, ipa_jump_func *jf2)
+{
+  if (jf1->type != jf2->type)
+    return false;
+
+  switch (jf1->type)
+    {
+    case IPA_JF_UNKNOWN:
+      break;
+    case IPA_JF_CONST:
+      {
+	tree cst1 = ipa_get_jf_constant (jf1);
+	tree cst2 = ipa_get_jf_constant (jf2);
+	if (!values_equal_for_ipcp_p (cst1, cst2))
+	  return false;
+
+	ipa_cst_ref_desc *rd1 = jfunc_rdesc_usable (jf1);
+	ipa_cst_ref_desc *rd2 = jfunc_rdesc_usable (jf2);
+	if (rd1 && rd2)
+	  {
+	    gcc_assert (rd1->refcount == 1
+			&& rd2->refcount == 1);
+	    gcc_assert (!rd1->next_duplicate && !rd2->next_duplicate);
+	  }
+	else if (rd1)
+	  return false;
+	else if (rd2)
+	  return false;
+      }
+      break;
+    case IPA_JF_PASS_THROUGH:
+      {
+	ipa_pass_through_data *ipt1 = &jf1->value.pass_through;
+	ipa_pass_through_data *ipt2 = &jf2->value.pass_through;
+	if (!ipa_agg_pass_through_jf_equivalent_p (ipt1, ipt2, false))
+	  return false;
+      }
+      break;
+    case IPA_JF_ANCESTOR:
+      {
+	ipa_ancestor_jf_data *ia1 = &jf1->value.ancestor;
+	ipa_ancestor_jf_data *ia2 = &jf2->value.ancestor;
+
+	if (ia1->formal_id != ia2->formal_id
+	    || ia1->agg_preserved != ia2->agg_preserved
+	    || ia1->keep_null != ia2->keep_null
+	    || ia1->offset != ia2->offset)
+	  return false;
+      }
+      break;
+    default:
+      gcc_unreachable ();
+    }
+
+  if (((jf1->m_vr != nullptr) != (jf2->m_vr != nullptr))
+      || (jf1->m_vr && !jf1->m_vr->equal_p (*jf2->m_vr)))
+    return false;
+
+  unsigned alen = vec_safe_length (jf1->agg.items);
+  if (vec_safe_length (jf2->agg.items) != alen)
+    return false;
+
+  if (!alen)
+    return true;
+
+  if (jf1->agg.by_ref != jf2->agg.by_ref)
+    return false;
+
+  for (unsigned i = 0 ; i < alen; i++)
+    if (!ipa_agg_jump_functions_equivalent_p (&(*jf1->agg.items)[i],
+					      &(*jf2->agg.items)[i]))
+      return false;
+
+  return true;
+}
+
 #include "gt-ipa-prop.h"
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index ee3c0006add..93d1b87b1f7 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -308,6 +308,7 @@  public:
   tree type () const { return m_type; }
   void get_vrange (Value_Range &) const;
   bool equal_p (const vrange &) const;
+  bool equal_p (const ipa_vr &) const;
   const vrange_storage *storage () const { return m_storage; }
   void streamer_read (lto_input_block *, class data_in *);
   void streamer_write (output_block *) const;
@@ -1278,5 +1279,7 @@  ipa_range_set_and_normalize (vrange &r, tree val)
 
 bool ipa_return_value_range (Value_Range &range, tree decl);
 void ipa_record_return_value_range (Value_Range val);
+bool ipa_jump_functions_equivalent_p (ipa_jump_func *jf1, ipa_jump_func *jf2);
+
 
 #endif /* IPA_PROP_H */
diff --git a/gcc/testsuite/gcc.dg/lto/pr113907_0.c b/gcc/testsuite/gcc.dg/lto/pr113907_0.c
new file mode 100644
index 00000000000..3c4dd475c01
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr113907_0.c
@@ -0,0 +1,18 @@ 
+/* { dg-lto-do run } */
+/* { dg-lto-options {{-O3 -flto}} }  */
+
+struct bar {int a;};
+struct foo {int a;};
+struct barp {struct bar *f; struct bar *g;};
+extern struct foo **ptr;
+int test2 (void *);
+int test3 (void *);
+int
+testb(void)
+{
+  struct bar *fp;
+  test2 ((void *)&fp);
+  fp = (void *) 0;
+  (*ptr)++;
+  test3 ((void *)&fp);
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr113907_1.c b/gcc/testsuite/gcc.dg/lto/pr113907_1.c
new file mode 100644
index 00000000000..1c48bfd83a4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr113907_1.c
@@ -0,0 +1,35 @@ 
+__attribute__((used)) int val,val2 = 1;
+
+struct foo {int a;};
+
+struct foo **ptr;
+
+__attribute__ ((noipa))
+int
+test2 (void *a)
+{
+  ptr = (struct foo **)a;
+}
+int test3 (void *a);
+
+int
+test(void)
+{
+  struct foo *fp;
+  test2 ((void *)&fp);
+  fp = (void *) 0;
+  (*ptr)++;
+  test3 ((void *)&fp);
+}
+
+int testb (void);
+
+int
+main()
+{
+  for (int i = 0; i < val2; i++)
+  if (val)
+    testb ();
+  else
+    test();
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr113907_2.c b/gcc/testsuite/gcc.dg/lto/pr113907_2.c
new file mode 100644
index 00000000000..172e86e8b0b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr113907_2.c
@@ -0,0 +1,11 @@ 
+/* { dg-options "-O3 -flto -fno-strict-aliasing" }  */
+
+__attribute__ ((noinline))
+int
+test3 (void *a)
+{
+  if (!*(void **)a)
+          __builtin_abort ();
+  return 0;
+}
+