[RFC] operand_equal_p with valueization

classic Classic list List threaded Threaded
58 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[RFC] operand_equal_p with valueization

Jan Hubicka-2
Hi,
with aliasing sanity checks I got burnt again with ipa-icf-gimple's
compare_operand doing alias set checks on all types it ever trips across.

I always tought that we do not need two equality testers - operand_equal_p and
compare_operand and given that it turns out to be non-trivial to fix issues in
compare_operand I decided to go ahead with plan to unify them.
I think operand_equal_p is better code base to start from since it is more
tested and knows more special cases.

The basic idea is to add valeize hook similar way as other folders do that
allows to do magic inside the recursive comparsion. I.e. declare two trees
equal if they are not (i.e. SSA_NAMES from differen functions).  I think it
should be useful for GVN/VRP/CCP too to improve folding of conditionals.

After trying out the hook and figuring out that ipa-icf does not have
global context to hold its tables, I dedcided that maybe more C++ way
is to have comparsion class that can be derived an adjusted for other
needs.

The following patch is a first step.  If it is considered sane, I will
continue by moving the code to one place - ipa-icf-gimple or special
ipa-icf-op.c. I will also recover Martin's diagnostics that is useful
to debug why things are considered different.

Also the code should be C++ized, for example it should use true/false
instead 0/1.

I think also the iterative hashing should be together with operand_equal_p
implementation because these two must match as I found with merging the two
cases that ipa-icf-gimple gets right and fold-const wrong - OBJ_TYPE_REF,
CONSTRUCTOR and PARM_DECL.

Finally I think we want compare_gimple class that does the same for
gimple and is independent of rest of the ipa-icf that may be better
suitable for stuff like tail merging.

The patch bootstraps/regtests ppc64-linux, but I think it is not quite
mainline ready as it noticeably reduces number of equivalences found.
I will need to debug why that happens, but I am sending it as an RFC for
the basic concept and interfaces.

Honza

        * fold-const.c (operand_equal_p): Reorg to wrapper for
        (operand_compare::operand_equal_p): This function; add
        support for valueization, add missing type matching for
        OBJ_TYPE_REF, CONSTRUCTOR; relax matching of PARM_DECL.
        (operand_compare::operand_equal_valueize): New.
        * fold-const.h (operand_equal_p): Update prototype.
        (class operand_compare): New class.
        * ipa-icf-gimple.c (func_checker::operand_equal_valueize): Break
        ipa-icf specific bits out from ...
        (func_checker::compare_operand): ... here; remove most of generic
        handling and use operand_compare class.
        * ipa-icf-gimple.h (operand_compare): New.
        * ipa-icf.c (sem_function::equals_private): Arrange CFUN to be up to
        date so we operand_equal_p works well for flag_devirtualize.
Index: fold-const.c
===================================================================
--- fold-const.c (revision 223500)
+++ fold-const.c (working copy)
@@ -2716,8 +2730,9 @@ combine_comparisons (location_t loc,
    are considered the same.  It is used when the caller has other ways
    to ensure that global memory is unchanged in between.  */
 
-int
-operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
+bool
+operand_compare::operand_equal_p (const_tree arg0, const_tree arg1,
+  unsigned int flags)
 {
   /* If either is ERROR_MARK, they aren't equal.  */
   if (TREE_CODE (arg0) == ERROR_MARK || TREE_CODE (arg1) == ERROR_MARK
@@ -2868,6 +2883,12 @@ operand_equal_p (const_tree arg0, const_
   if (flags & OEP_ONLY_CONST)
     return 0;
 
+  int val = operand_equal_valueize (arg0, arg1, flags);
+  if (val == 1)
+    return 1;
+  if (val == 0)
+    return 0;
+
 /* Define macros to test an operand from arg0 and arg1 for equality and a
    variant that allows null and views null as being different from any
    non-null value.  In the latter case, if either is null, the both
@@ -3104,12 +3174,50 @@ operand_equal_p (const_tree arg0, const_
       && DECL_FUNCTION_CODE (arg0) == DECL_FUNCTION_CODE (arg1));
 
     default:
       return 0;
     }
 
 #undef OP_SAME
 #undef OP_SAME_WITH_NULL
 }
+
+/* Valueizer is a virtual method that allows to introduce extra equalities
+   that are not directly visible from the operand.
+   N1 means values are known to be equal, 0 means values are known to be different
+   -1 means that operand_equal_p should continue processing.  */
+int
+operand_compare::operand_equal_valueize (const_tree, const_tree, unsigned int)
+{
+  return -1;
+}
+
+/* Conveinece wrapper around operand_compare class because usually we do
+   not need to play with the valueizer.  */
+bool
+operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
+{
+  static operand_compare default_compare_instance;
+  return default_compare_instance.operand_equal_p (arg0, arg1, flags);
+}
 
 /* Similar to operand_equal_p, but see if ARG0 might have been made by
    shorten_compare from ARG1 when ARG1 was being compared with OTHER.
Index: fold-const.h
===================================================================
--- fold-const.h (revision 223500)
+++ fold-const.h (working copy)
@@ -85,7 +85,7 @@ extern void fold_defer_overflow_warnings
 extern void fold_undefer_overflow_warnings (bool, const_gimple, int);
 extern void fold_undefer_and_ignore_overflow_warnings (void);
 extern bool fold_deferring_overflow_warnings_p (void);
-extern int operand_equal_p (const_tree, const_tree, unsigned int);
+extern bool operand_equal_p (const_tree, const_tree, unsigned int flags = 0);
 extern int multiple_of_p (tree, const_tree, const_tree);
 #define omit_one_operand(T1,T2,T3)\
    omit_one_operand_loc (UNKNOWN_LOCATION, T1, T2, T3)
@@ -189,4 +189,20 @@ extern tree fold_build_pointer_plus_hwi_
 
 #define fold_build_pointer_plus_hwi(p,o) \
  fold_build_pointer_plus_hwi_loc (UNKNOWN_LOCATION, p, o)
+
+/* Class used to compare gimple operands.  */
+
+class operand_compare
+{
+public:
+  /* Return true if two operands are equal.  THe flags fields can be used
+     to specify OEP flags described above.   */
+  bool operand_equal_p (const_tree, const_tree, unsigned int flags = 0);
+private:
+  /* Valueizer can be used to make non-trivial equalities for expressions
+     that do not look same in isolation.
+     1 means values are known to be equal, 0 means values are known to be
+     different -1 means that operand_equal_p should continue processing.  */
+  virtual int operand_equal_valueize (const_tree, const_tree, unsigned int);
+};
 #endif // GCC_FOLD_CONST_H
Index: ipa-icf-gimple.c
===================================================================
--- ipa-icf-gimple.c (revision 223500)
+++ ipa-icf-gimple.c (working copy)
@@ -412,165 +428,57 @@ func_checker::compare_cst_or_decl (tree
     }
 }
 
-/* Function responsible for comparison of various operands T1 and T2.
-   If these components, from functions FUNC1 and FUNC2, are equal, true
-   is returned.  */
-
-bool
-func_checker::compare_operand (tree t1, tree t2)
+int
+func_checker::operand_equal_valueize (const_tree ct1, const_tree ct2, unsigned int)
 {
-  tree x1, x2, y1, y2, z1, z2;
+  tree t1 = const_cast <tree> (ct1);
+  tree t2 = const_cast <tree> (ct2);
   bool ret;
 
-  if (!t1 && !t2)
-    return true;
-  else if (!t1 || !t2)
-    return false;
-
-  tree tt1 = TREE_TYPE (t1);
-  tree tt2 = TREE_TYPE (t2);
-
-  if (!func_checker::compatible_types_p (tt1, tt2))
-    return false;
-
-  if (TREE_CODE (t1) != TREE_CODE (t2))
-    return return_false ();
-
   switch (TREE_CODE (t1))
     {
-    case CONSTRUCTOR:
-      {
- unsigned length1 = vec_safe_length (CONSTRUCTOR_ELTS (t1));
- unsigned length2 = vec_safe_length (CONSTRUCTOR_ELTS (t2));
-
- if (length1 != length2)
-  return return_false ();
-
- for (unsigned i = 0; i < length1; i++)
-  if (!compare_operand (CONSTRUCTOR_ELT (t1, i)->value,
- CONSTRUCTOR_ELT (t2, i)->value))
-    return return_false();
-
- return true;
-      }
-    case ARRAY_REF:
-    case ARRAY_RANGE_REF:
-      /* First argument is the array, second is the index.  */
-      x1 = TREE_OPERAND (t1, 0);
-      x2 = TREE_OPERAND (t2, 0);
-      y1 = TREE_OPERAND (t1, 1);
-      y2 = TREE_OPERAND (t2, 1);
-
-      if (!compare_operand (array_ref_low_bound (t1),
-    array_ref_low_bound (t2)))
- return return_false_with_msg ("");
-      if (!compare_operand (array_ref_element_size (t1),
-    array_ref_element_size (t2)))
- return return_false_with_msg ("");
-
-      if (!compare_operand (x1, x2))
- return return_false_with_msg ("");
-      return compare_operand (y1, y2);
-    case MEM_REF:
-      {
- x1 = TREE_OPERAND (t1, 0);
- x2 = TREE_OPERAND (t2, 0);
- y1 = TREE_OPERAND (t1, 1);
- y2 = TREE_OPERAND (t2, 1);
-
- /* See if operand is an memory access (the test originate from
- gimple_load_p).
-
- In this case the alias set of the function being replaced must
- be subset of the alias set of the other function.  At the moment
- we seek for equivalency classes, so simply require inclussion in
- both directions.  */
-
- if (!func_checker::compatible_types_p (TREE_TYPE (x1), TREE_TYPE (x2)))
-  return return_false ();
-
- if (!compare_operand (x1, x2))
-  return return_false_with_msg ("");
-
- /* Type of the offset on MEM_REF does not matter.  */
- return wi::to_offset  (y1) == wi::to_offset  (y2);
-      }
-    case COMPONENT_REF:
-      {
- x1 = TREE_OPERAND (t1, 0);
- x2 = TREE_OPERAND (t2, 0);
- y1 = TREE_OPERAND (t1, 1);
- y2 = TREE_OPERAND (t2, 1);
-
- ret = compare_operand (x1, x2)
-      && compare_cst_or_decl (y1, y2);
-
- return return_with_debug (ret);
-      }
-    /* Virtual table call.  */
-    case OBJ_TYPE_REF:
-      {
- if (!compare_ssa_name (OBJ_TYPE_REF_EXPR (t1), OBJ_TYPE_REF_EXPR (t2)))
-  return return_false ();
- if (opt_for_fn (m_source_func_decl, flag_devirtualize)
-    && virtual_method_call_p (t1))
-  {
-    if (tree_to_uhwi (OBJ_TYPE_REF_TOKEN (t1))
- != tree_to_uhwi (OBJ_TYPE_REF_TOKEN (t2)))
-      return return_false_with_msg ("OBJ_TYPE_REF token mismatch");
-    if (!types_same_for_odr (obj_type_ref_class (t1),
-     obj_type_ref_class (t2)))
-      return return_false_with_msg ("OBJ_TYPE_REF OTR type mismatch");
-    if (!compare_operand (OBJ_TYPE_REF_OBJECT (t1),
-  OBJ_TYPE_REF_OBJECT (t2)))
-      return return_false_with_msg ("OBJ_TYPE_REF object mismatch");
-  }
-
- return return_with_debug (true);
-      }
-    case IMAGPART_EXPR:
-    case REALPART_EXPR:
-    case ADDR_EXPR:
+    case FUNCTION_DECL:
+      /* All function decls are in the symbol table and known to match
+ before we start comparing bodies.  */
+      return true;
+    case VAR_DECL:
+      return return_with_debug (compare_variable_decl (t1, t2));
+    case LABEL_DECL:
       {
- x1 = TREE_OPERAND (t1, 0);
- x2 = TREE_OPERAND (t2, 0);
+ int *bb1 = m_label_bb_map.get (t1);
+ int *bb2 = m_label_bb_map.get (t2);
 
- ret = compare_operand (x1, x2);
- return return_with_debug (ret);
+ return return_with_debug (*bb1 == *bb2);
       }
-    case BIT_FIELD_REF:
+    case PARM_DECL:
+    case RESULT_DECL:
+    case CONST_DECL:
       {
- x1 = TREE_OPERAND (t1, 0);
- x2 = TREE_OPERAND (t2, 0);
- y1 = TREE_OPERAND (t1, 1);
- y2 = TREE_OPERAND (t2, 1);
- z1 = TREE_OPERAND (t1, 2);
- z2 = TREE_OPERAND (t2, 2);
-
- ret = compare_operand (x1, x2)
-      && compare_cst_or_decl (y1, y2)
-      && compare_cst_or_decl (z1, z2);
-
+ ret = compare_decl (t1, t2);
  return return_with_debug (ret);
       }
     case SSA_NAME:
- return compare_ssa_name (t1, t2);
-    case INTEGER_CST:
-    case COMPLEX_CST:
-    case VECTOR_CST:
-    case STRING_CST:
-    case REAL_CST:
-    case FUNCTION_DECL:
-    case VAR_DECL:
-    case FIELD_DECL:
-    case LABEL_DECL:
-    case PARM_DECL:
-    case RESULT_DECL:
-    case CONST_DECL:
-      return compare_cst_or_decl (t1, t2);
+      return compare_ssa_name (t1, t2);
     default:
-      return return_false_with_msg ("Unknown TREE code reached");
+      break;
     }
+  return -1;
+}
+
+/* Function responsible for comparison of various operands T1 and T2.
+   If these components, from functions FUNC1 and FUNC2, are equal, true
+   is returned.  */
+
+bool
+func_checker::compare_operand (tree t1, tree t2)
+{
+  if (!t1 && !t2)
+    return true;
+  else if (!t1 || !t2)
+    return false;
+  if (operand_equal_p (t1, t2, 0))
+    return true;
+  return return_false_with_msg ("operand_equal_p failed");
 }
 
 /* Compares two tree list operands T1 and T2 and returns true if these
Index: ipa-icf-gimple.h
===================================================================
--- ipa-icf-gimple.h (revision 223500)
+++ ipa-icf-gimple.h (working copy)
@@ -127,7 +127,7 @@ public:
 
 /* A class aggregating all connections and semantic equivalents
    for a given pair of semantic function candidates.  */
-class func_checker
+class func_checker : operand_compare
 {
 public:
   /* Initialize internal structures for a given SOURCE_FUNC_DECL and
@@ -143,7 +143,7 @@ public:
  hash_set<symtab_node *> *ignored_target_nodes = NULL);
 
   /* Memory release routine.  */
-  ~func_checker();
+  virtual ~func_checker();
 
   /* Function visits all gimple labels and creates corresponding
      mapping between basic blocks and labels.  */
@@ -273,6 +273,8 @@ private:
 
   /* Flag if ignore labels in comparison.  */
   bool m_ignore_labels;
+
+  virtual int operand_equal_valueize (const_tree, const_tree, unsigned int);
 };
 
 } // ipa_icf_gimple namespace
Index: ipa-icf.c
===================================================================
--- ipa-icf.c (revision 223500)
+++ ipa-icf.c (working copy)
@@ -953,9 +953,14 @@ sem_function::equals_private (sem_item *
     }
 
   /* Checking all basic blocks.  */
+  push_cfun (DECL_STRUCT_FUNCTION (decl));
   for (unsigned i = 0; i < bb_sorted.length (); ++i)
     if(!m_checker->compare_bb (bb_sorted[i], m_compared_func->bb_sorted[i]))
-      return return_false();
+      {
+ pop_cfun ();
+        return return_false();
+      }
+  pop_cfun ();
 
   dump_message ("All BBs are equal\n");
 
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] operand_equal_p with valueization

Richard Biener
On Fri, 22 May 2015, Jan Hubicka wrote:

> Hi,
> with aliasing sanity checks I got burnt again with ipa-icf-gimple's
> compare_operand doing alias set checks on all types it ever trips across.
>
> I always tought that we do not need two equality testers - operand_equal_p and
> compare_operand and given that it turns out to be non-trivial to fix issues in
> compare_operand I decided to go ahead with plan to unify them.
> I think operand_equal_p is better code base to start from since it is more
> tested and knows more special cases.
>
> The basic idea is to add valeize hook similar way as other folders do that
> allows to do magic inside the recursive comparsion. I.e. declare two trees
> equal if they are not (i.e. SSA_NAMES from differen functions).  I think it
> should be useful for GVN/VRP/CCP too to improve folding of conditionals.
>
> After trying out the hook and figuring out that ipa-icf does not have
> global context to hold its tables, I dedcided that maybe more C++ way
> is to have comparsion class that can be derived an adjusted for other
> needs.
>
> The following patch is a first step.  If it is considered sane, I will
> continue by moving the code to one place - ipa-icf-gimple or special
> ipa-icf-op.c. I will also recover Martin's diagnostics that is useful
> to debug why things are considered different.
>
> Also the code should be C++ized, for example it should use true/false
> instead 0/1.
>
> I think also the iterative hashing should be together with operand_equal_p
> implementation because these two must match as I found with merging the two
> cases that ipa-icf-gimple gets right and fold-const wrong - OBJ_TYPE_REF,
> CONSTRUCTOR and PARM_DECL.
>
> Finally I think we want compare_gimple class that does the same for
> gimple and is independent of rest of the ipa-icf that may be better
> suitable for stuff like tail merging.
>
> The patch bootstraps/regtests ppc64-linux, but I think it is not quite
> mainline ready as it noticeably reduces number of equivalences found.
> I will need to debug why that happens, but I am sending it as an RFC for
> the basic concept and interfaces.

I think for the case of IPA ICF it would be better to address the
issue that it cannot do merging of functions with TBAA conflicts.
That is, drop that TBAA code from IPA ICF and arrange for the
IPA inliner to inline original unmerged copies.  We were talking
about making the original nodes "inline clones" of the node that
eventually prevails, much similar to speculative inlining ones
(if I remember that suggestion from Martin correctly).

And no, I'm hesitant to change operand_equal_p too much.  It's
very much deep-rooted into GENERIC.

Richard.


> Honza
>
> * fold-const.c (operand_equal_p): Reorg to wrapper for
> (operand_compare::operand_equal_p): This function; add
> support for valueization, add missing type matching for
> OBJ_TYPE_REF, CONSTRUCTOR; relax matching of PARM_DECL.
> (operand_compare::operand_equal_valueize): New.
> * fold-const.h (operand_equal_p): Update prototype.
> (class operand_compare): New class.
> * ipa-icf-gimple.c (func_checker::operand_equal_valueize): Break
> ipa-icf specific bits out from ...
> (func_checker::compare_operand): ... here; remove most of generic
> handling and use operand_compare class.
> * ipa-icf-gimple.h (operand_compare): New.
> * ipa-icf.c (sem_function::equals_private): Arrange CFUN to be up to
> date so we operand_equal_p works well for flag_devirtualize.
> Index: fold-const.c
> ===================================================================
> --- fold-const.c (revision 223500)
> +++ fold-const.c (working copy)
> @@ -2716,8 +2730,9 @@ combine_comparisons (location_t loc,
>     are considered the same.  It is used when the caller has other ways
>     to ensure that global memory is unchanged in between.  */
>  
> -int
> -operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
> +bool
> +operand_compare::operand_equal_p (const_tree arg0, const_tree arg1,
> +  unsigned int flags)
>  {
>    /* If either is ERROR_MARK, they aren't equal.  */
>    if (TREE_CODE (arg0) == ERROR_MARK || TREE_CODE (arg1) == ERROR_MARK
> @@ -2868,6 +2883,12 @@ operand_equal_p (const_tree arg0, const_
>    if (flags & OEP_ONLY_CONST)
>      return 0;
>  
> +  int val = operand_equal_valueize (arg0, arg1, flags);
> +  if (val == 1)
> +    return 1;
> +  if (val == 0)
> +    return 0;
> +
>  /* Define macros to test an operand from arg0 and arg1 for equality and a
>     variant that allows null and views null as being different from any
>     non-null value.  In the latter case, if either is null, the both
> @@ -3104,12 +3174,50 @@ operand_equal_p (const_tree arg0, const_
>        && DECL_FUNCTION_CODE (arg0) == DECL_FUNCTION_CODE (arg1));
>  
>      default:
>        return 0;
>      }
>  
>  #undef OP_SAME
>  #undef OP_SAME_WITH_NULL
>  }
> +
> +/* Valueizer is a virtual method that allows to introduce extra equalities
> +   that are not directly visible from the operand.
> +   N1 means values are known to be equal, 0 means values are known to be different
> +   -1 means that operand_equal_p should continue processing.  */
> +int
> +operand_compare::operand_equal_valueize (const_tree, const_tree, unsigned int)
> +{
> +  return -1;
> +}
> +
> +/* Conveinece wrapper around operand_compare class because usually we do
> +   not need to play with the valueizer.  */
> +bool
> +operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
> +{
> +  static operand_compare default_compare_instance;
> +  return default_compare_instance.operand_equal_p (arg0, arg1, flags);
> +}
>  
>  /* Similar to operand_equal_p, but see if ARG0 might have been made by
>     shorten_compare from ARG1 when ARG1 was being compared with OTHER.
> Index: fold-const.h
> ===================================================================
> --- fold-const.h (revision 223500)
> +++ fold-const.h (working copy)
> @@ -85,7 +85,7 @@ extern void fold_defer_overflow_warnings
>  extern void fold_undefer_overflow_warnings (bool, const_gimple, int);
>  extern void fold_undefer_and_ignore_overflow_warnings (void);
>  extern bool fold_deferring_overflow_warnings_p (void);
> -extern int operand_equal_p (const_tree, const_tree, unsigned int);
> +extern bool operand_equal_p (const_tree, const_tree, unsigned int flags = 0);
>  extern int multiple_of_p (tree, const_tree, const_tree);
>  #define omit_one_operand(T1,T2,T3)\
>     omit_one_operand_loc (UNKNOWN_LOCATION, T1, T2, T3)
> @@ -189,4 +189,20 @@ extern tree fold_build_pointer_plus_hwi_
>  
>  #define fold_build_pointer_plus_hwi(p,o) \
>   fold_build_pointer_plus_hwi_loc (UNKNOWN_LOCATION, p, o)
> +
> +/* Class used to compare gimple operands.  */
> +
> +class operand_compare
> +{
> +public:
> +  /* Return true if two operands are equal.  THe flags fields can be used
> +     to specify OEP flags described above.   */
> +  bool operand_equal_p (const_tree, const_tree, unsigned int flags = 0);
> +private:
> +  /* Valueizer can be used to make non-trivial equalities for expressions
> +     that do not look same in isolation.
> +     1 means values are known to be equal, 0 means values are known to be
> +     different -1 means that operand_equal_p should continue processing.  */
> +  virtual int operand_equal_valueize (const_tree, const_tree, unsigned int);
> +};
>  #endif // GCC_FOLD_CONST_H
> Index: ipa-icf-gimple.c
> ===================================================================
> --- ipa-icf-gimple.c (revision 223500)
> +++ ipa-icf-gimple.c (working copy)
> @@ -412,165 +428,57 @@ func_checker::compare_cst_or_decl (tree
>      }
>  }
>  
> -/* Function responsible for comparison of various operands T1 and T2.
> -   If these components, from functions FUNC1 and FUNC2, are equal, true
> -   is returned.  */
> -
> -bool
> -func_checker::compare_operand (tree t1, tree t2)
> +int
> +func_checker::operand_equal_valueize (const_tree ct1, const_tree ct2, unsigned int)
>  {
> -  tree x1, x2, y1, y2, z1, z2;
> +  tree t1 = const_cast <tree> (ct1);
> +  tree t2 = const_cast <tree> (ct2);
>    bool ret;
>  
> -  if (!t1 && !t2)
> -    return true;
> -  else if (!t1 || !t2)
> -    return false;
> -
> -  tree tt1 = TREE_TYPE (t1);
> -  tree tt2 = TREE_TYPE (t2);
> -
> -  if (!func_checker::compatible_types_p (tt1, tt2))
> -    return false;
> -
> -  if (TREE_CODE (t1) != TREE_CODE (t2))
> -    return return_false ();
> -
>    switch (TREE_CODE (t1))
>      {
> -    case CONSTRUCTOR:
> -      {
> - unsigned length1 = vec_safe_length (CONSTRUCTOR_ELTS (t1));
> - unsigned length2 = vec_safe_length (CONSTRUCTOR_ELTS (t2));
> -
> - if (length1 != length2)
> -  return return_false ();
> -
> - for (unsigned i = 0; i < length1; i++)
> -  if (!compare_operand (CONSTRUCTOR_ELT (t1, i)->value,
> - CONSTRUCTOR_ELT (t2, i)->value))
> -    return return_false();
> -
> - return true;
> -      }
> -    case ARRAY_REF:
> -    case ARRAY_RANGE_REF:
> -      /* First argument is the array, second is the index.  */
> -      x1 = TREE_OPERAND (t1, 0);
> -      x2 = TREE_OPERAND (t2, 0);
> -      y1 = TREE_OPERAND (t1, 1);
> -      y2 = TREE_OPERAND (t2, 1);
> -
> -      if (!compare_operand (array_ref_low_bound (t1),
> -    array_ref_low_bound (t2)))
> - return return_false_with_msg ("");
> -      if (!compare_operand (array_ref_element_size (t1),
> -    array_ref_element_size (t2)))
> - return return_false_with_msg ("");
> -
> -      if (!compare_operand (x1, x2))
> - return return_false_with_msg ("");
> -      return compare_operand (y1, y2);
> -    case MEM_REF:
> -      {
> - x1 = TREE_OPERAND (t1, 0);
> - x2 = TREE_OPERAND (t2, 0);
> - y1 = TREE_OPERAND (t1, 1);
> - y2 = TREE_OPERAND (t2, 1);
> -
> - /* See if operand is an memory access (the test originate from
> - gimple_load_p).
> -
> - In this case the alias set of the function being replaced must
> - be subset of the alias set of the other function.  At the moment
> - we seek for equivalency classes, so simply require inclussion in
> - both directions.  */
> -
> - if (!func_checker::compatible_types_p (TREE_TYPE (x1), TREE_TYPE (x2)))
> -  return return_false ();
> -
> - if (!compare_operand (x1, x2))
> -  return return_false_with_msg ("");
> -
> - /* Type of the offset on MEM_REF does not matter.  */
> - return wi::to_offset  (y1) == wi::to_offset  (y2);
> -      }
> -    case COMPONENT_REF:
> -      {
> - x1 = TREE_OPERAND (t1, 0);
> - x2 = TREE_OPERAND (t2, 0);
> - y1 = TREE_OPERAND (t1, 1);
> - y2 = TREE_OPERAND (t2, 1);
> -
> - ret = compare_operand (x1, x2)
> -      && compare_cst_or_decl (y1, y2);
> -
> - return return_with_debug (ret);
> -      }
> -    /* Virtual table call.  */
> -    case OBJ_TYPE_REF:
> -      {
> - if (!compare_ssa_name (OBJ_TYPE_REF_EXPR (t1), OBJ_TYPE_REF_EXPR (t2)))
> -  return return_false ();
> - if (opt_for_fn (m_source_func_decl, flag_devirtualize)
> -    && virtual_method_call_p (t1))
> -  {
> -    if (tree_to_uhwi (OBJ_TYPE_REF_TOKEN (t1))
> - != tree_to_uhwi (OBJ_TYPE_REF_TOKEN (t2)))
> -      return return_false_with_msg ("OBJ_TYPE_REF token mismatch");
> -    if (!types_same_for_odr (obj_type_ref_class (t1),
> -     obj_type_ref_class (t2)))
> -      return return_false_with_msg ("OBJ_TYPE_REF OTR type mismatch");
> -    if (!compare_operand (OBJ_TYPE_REF_OBJECT (t1),
> -  OBJ_TYPE_REF_OBJECT (t2)))
> -      return return_false_with_msg ("OBJ_TYPE_REF object mismatch");
> -  }
> -
> - return return_with_debug (true);
> -      }
> -    case IMAGPART_EXPR:
> -    case REALPART_EXPR:
> -    case ADDR_EXPR:
> +    case FUNCTION_DECL:
> +      /* All function decls are in the symbol table and known to match
> + before we start comparing bodies.  */
> +      return true;
> +    case VAR_DECL:
> +      return return_with_debug (compare_variable_decl (t1, t2));
> +    case LABEL_DECL:
>        {
> - x1 = TREE_OPERAND (t1, 0);
> - x2 = TREE_OPERAND (t2, 0);
> + int *bb1 = m_label_bb_map.get (t1);
> + int *bb2 = m_label_bb_map.get (t2);
>  
> - ret = compare_operand (x1, x2);
> - return return_with_debug (ret);
> + return return_with_debug (*bb1 == *bb2);
>        }
> -    case BIT_FIELD_REF:
> +    case PARM_DECL:
> +    case RESULT_DECL:
> +    case CONST_DECL:
>        {
> - x1 = TREE_OPERAND (t1, 0);
> - x2 = TREE_OPERAND (t2, 0);
> - y1 = TREE_OPERAND (t1, 1);
> - y2 = TREE_OPERAND (t2, 1);
> - z1 = TREE_OPERAND (t1, 2);
> - z2 = TREE_OPERAND (t2, 2);
> -
> - ret = compare_operand (x1, x2)
> -      && compare_cst_or_decl (y1, y2)
> -      && compare_cst_or_decl (z1, z2);
> -
> + ret = compare_decl (t1, t2);
>   return return_with_debug (ret);
>        }
>      case SSA_NAME:
> - return compare_ssa_name (t1, t2);
> -    case INTEGER_CST:
> -    case COMPLEX_CST:
> -    case VECTOR_CST:
> -    case STRING_CST:
> -    case REAL_CST:
> -    case FUNCTION_DECL:
> -    case VAR_DECL:
> -    case FIELD_DECL:
> -    case LABEL_DECL:
> -    case PARM_DECL:
> -    case RESULT_DECL:
> -    case CONST_DECL:
> -      return compare_cst_or_decl (t1, t2);
> +      return compare_ssa_name (t1, t2);
>      default:
> -      return return_false_with_msg ("Unknown TREE code reached");
> +      break;
>      }
> +  return -1;
> +}
> +
> +/* Function responsible for comparison of various operands T1 and T2.
> +   If these components, from functions FUNC1 and FUNC2, are equal, true
> +   is returned.  */
> +
> +bool
> +func_checker::compare_operand (tree t1, tree t2)
> +{
> +  if (!t1 && !t2)
> +    return true;
> +  else if (!t1 || !t2)
> +    return false;
> +  if (operand_equal_p (t1, t2, 0))
> +    return true;
> +  return return_false_with_msg ("operand_equal_p failed");
>  }
>  
>  /* Compares two tree list operands T1 and T2 and returns true if these
> Index: ipa-icf-gimple.h
> ===================================================================
> --- ipa-icf-gimple.h (revision 223500)
> +++ ipa-icf-gimple.h (working copy)
> @@ -127,7 +127,7 @@ public:
>  
>  /* A class aggregating all connections and semantic equivalents
>     for a given pair of semantic function candidates.  */
> -class func_checker
> +class func_checker : operand_compare
>  {
>  public:
>    /* Initialize internal structures for a given SOURCE_FUNC_DECL and
> @@ -143,7 +143,7 @@ public:
>   hash_set<symtab_node *> *ignored_target_nodes = NULL);
>  
>    /* Memory release routine.  */
> -  ~func_checker();
> +  virtual ~func_checker();
>  
>    /* Function visits all gimple labels and creates corresponding
>       mapping between basic blocks and labels.  */
> @@ -273,6 +273,8 @@ private:
>  
>    /* Flag if ignore labels in comparison.  */
>    bool m_ignore_labels;
> +
> +  virtual int operand_equal_valueize (const_tree, const_tree, unsigned int);
>  };
>  
>  } // ipa_icf_gimple namespace
> Index: ipa-icf.c
> ===================================================================
> --- ipa-icf.c (revision 223500)
> +++ ipa-icf.c (working copy)
> @@ -953,9 +953,14 @@ sem_function::equals_private (sem_item *
>      }
>  
>    /* Checking all basic blocks.  */
> +  push_cfun (DECL_STRUCT_FUNCTION (decl));
>    for (unsigned i = 0; i < bb_sorted.length (); ++i)
>      if(!m_checker->compare_bb (bb_sorted[i], m_compared_func->bb_sorted[i]))
> -      return return_false();
> +      {
> + pop_cfun ();
> +        return return_false();
> +      }
> +  pop_cfun ();
>  
>    dump_message ("All BBs are equal\n");
>  
>
>

--
Richard Biener <[hidden email]>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] operand_equal_p with valueization

Jan Hubicka-2
>
> And no, I'm hesitant to change operand_equal_p too much.  It's
> very much deep-rooted into GENERIC.

OK, as another option, i can bring relevant logic from operand_equal_p
to ipa-icf and separate it into the compare_operand class like I did.
Use it in ipa-icf-gimple now and we can slowly turn other uses of
operand_equal into the compare_operand users in middle end.

I agree that operand_equal is bit crazy code and it does not handle quite few
things we could do at gimple.  I have nothing against going this direction.
(after all I do not like touching fold-const much becuase it works on generic,
gimple and FE non-generic and it is not well specified what it should do)

Honza
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] operand_equal_p with valueization

Richard Biener
On Fri, 22 May 2015, Jan Hubicka wrote:

> >
> > And no, I'm hesitant to change operand_equal_p too much.  It's
> > very much deep-rooted into GENERIC.
>
> OK, as another option, i can bring relevant logic from operand_equal_p
> to ipa-icf and separate it into the compare_operand class like I did.
> Use it in ipa-icf-gimple now and we can slowly turn other uses of
> operand_equal into the compare_operand users in middle end.
>
> I agree that operand_equal is bit crazy code and it does not handle quite few
> things we could do at gimple.  I have nothing against going this direction.
> (after all I do not like touching fold-const much becuase it works on generic,
> gimple and FE non-generic and it is not well specified what it should do)

Yes, I've played with the idea of a GIMPLE specific operand_equal_p
multiple times but then the changes required to operand_equal_p were
small all the times.  And having one piece of code that does sth is
always good ...

We might turn operand_equal_p to a "worker" (template?) that
operand_equal_p and gimple_operand_equal_p can share (with an extra
flag whether to turn on GIMPLE stuff and/or valueization).  And
then simply provide explicit instantiations for the original
operand_equal_p and a new gimple_operand_equal_p.

Of course we'll only know if we like that when seeing a patch that
does this ;0)

Richard.
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] operand_equal_p with valueization

Jan Hubicka-2
> On Fri, 22 May 2015, Jan Hubicka wrote:
>
> > >
> > > And no, I'm hesitant to change operand_equal_p too much.  It's
> > > very much deep-rooted into GENERIC.
> >
> > OK, as another option, i can bring relevant logic from operand_equal_p
> > to ipa-icf and separate it into the compare_operand class like I did.
> > Use it in ipa-icf-gimple now and we can slowly turn other uses of
> > operand_equal into the compare_operand users in middle end.
> >
> > I agree that operand_equal is bit crazy code and it does not handle quite few
> > things we could do at gimple.  I have nothing against going this direction.
> > (after all I do not like touching fold-const much becuase it works on generic,
> > gimple and FE non-generic and it is not well specified what it should do)
>
> Yes, I've played with the idea of a GIMPLE specific operand_equal_p
> multiple times but then the changes required to operand_equal_p were
> small all the times.  And having one piece of code that does sth is
> always good ...
>
> We might turn operand_equal_p to a "worker" (template?) that

Hmm, OK that is precisely what I was shooting for by this patch.  I went by
wrapping it to a class with valueize helper.  It can be template, too, just it
semed that having the single valueize function lets me do everything I need
without actually needing to duplicate the code.

I can get around templatizing it.  Do you have some outline what interface
would seem more fit>

> operand_equal_p and gimple_operand_equal_p can share (with an extra
> flag whether to turn on GIMPLE stuff and/or valueization).  And
> then simply provide explicit instantiations for the original
> operand_equal_p and a new gimple_operand_equal_p.
>
> Of course we'll only know if we like that when seeing a patch that
> does this ;0)
>
> Richard.
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] operand_equal_p with valueization

Richard Biener
On Tue, 26 May 2015, Jan Hubicka wrote:

> > On Fri, 22 May 2015, Jan Hubicka wrote:
> >
> > > >
> > > > And no, I'm hesitant to change operand_equal_p too much.  It's
> > > > very much deep-rooted into GENERIC.
> > >
> > > OK, as another option, i can bring relevant logic from operand_equal_p
> > > to ipa-icf and separate it into the compare_operand class like I did.
> > > Use it in ipa-icf-gimple now and we can slowly turn other uses of
> > > operand_equal into the compare_operand users in middle end.
> > >
> > > I agree that operand_equal is bit crazy code and it does not handle quite few
> > > things we could do at gimple.  I have nothing against going this direction.
> > > (after all I do not like touching fold-const much becuase it works on generic,
> > > gimple and FE non-generic and it is not well specified what it should do)
> >
> > Yes, I've played with the idea of a GIMPLE specific operand_equal_p
> > multiple times but then the changes required to operand_equal_p were
> > small all the times.  And having one piece of code that does sth is
> > always good ...
> >
> > We might turn operand_equal_p to a "worker" (template?) that
>
> Hmm, OK that is precisely what I was shooting for by this patch.  I went by
> wrapping it to a class with valueize helper.  It can be template, too, just it
> semed that having the single valueize function lets me do everything I need
> without actually needing to duplicate the code.
>
> I can get around templatizing it.  Do you have some outline what interface
> would seem more fit>

I was thinking about

template <bool with_valueize>
int
operand_equal_p_1 (const_tree arg0, const_tree arg1, unsigned int flags,
                   tree (*valueize)(tree))
{
#define VALUEIZE(op) (with_valueize && valueize) ? valueize (op) : op
...
}

and

extern template <>
int operand_equal_p_1<false> (const_tree arg0, const_tree arg1,
unsigned int flags,
                   tree (*valueize)(tree));
extern template <>
int operand_equal_p_1<true> (const_tree arg0, const_tree arg1,    
unsigned int flags,
                   tree (*valueize)(tree));

int
operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
{
  return operand_equal_p_1<false> (arg0, arg1, flags, NULL);
}

we don't want to make 'valueize' a template parameter (that is,
we don't want to put operand_equal_p_1 to fold-const.h).

Same with an eventual 'gimple_p' template parameter (which eventually
could simply be the same as the with_valueize one).

I'm playing with the idea to make match-and-simplify similar,
providing explicit specializations for "common" valueize callbacks.
As it always has a valueize callback I'd do it like

template <tree (*fixed_valueize)(tree)>
bool
gimple_simplify (code_helper *res_code, tree *res_ops,
                 gimple_seq *seq, tree (*valueize)(tree),
                 code_helper code, tree type, tree op0)
{
#define do_valueize(op) \
  fixed_valueize != (void *)-1 \
  ? (fixed_valueize ? fixed_valueize (op) : op) \
  : (valueize ? valueize (op) : op)
...
}

Richard.

> > operand_equal_p and gimple_operand_equal_p can share (with an extra
> > flag whether to turn on GIMPLE stuff and/or valueization).  And
> > then simply provide explicit instantiations for the original
> > operand_equal_p and a new gimple_operand_equal_p.
> >
> > Of course we'll only know if we like that when seeing a patch that
> > does this ;0)
> >
> > Richard.
>
>

--
Richard Biener <[hidden email]>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)
Reply | Threaded
Open this post in threaded view
|

[RFC] Re: [RFC] operand_equal_p with valueization

Martin Liška-2
Hi.

It's quite some time the discussion has started. Now is time for me to refresh IPA ICF
and I would like integrate operand_equal_p with what I currently have in ICF (::compare_operand).
I like the idea of a class that will provide both operand_equal_valueize and hash_operand_valueize.
These will be implemented in func_checker and can provide basically the same what Honza suggested.

Reading the thread, I noticed Richi would prefer to use something like:

template <bool with_valueize>
int
operand_equal_p_1 (const_tree arg0, const_tree arg1, unsigned int flags,
                   tree (*valueize)(tree))
{
#define VALUEIZE(op) (with_valueize && valueize) ? valueize (op) : op
...
}

To be honest, it looks to me only as an optimization which will fold call
to valueize in current operand_equal_p.

I'm sending a slightly tested pair of patches which does the abstraction
factoring and ICF adaptation.

I would expect a feedback before I'll prepare a proper fix.
Thanks,
Martin

0002-Integrate-that-for-IPA-ICF.patch (9K) Download Attachment
0001-Come-up-with-an-abstraction.patch (23K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/9] IPA ICF overhaul

Martin Liška-2
Hi.

It's some time I implemented first version of IPA ICF pass. Since that
I experienced more with the GCC internals and now is the right time
to do an overhaul.

Main motivation of changes is to share as many as possible in between
current operand_equal_p and ipa_icf::compare_operand. That's achieved
by a new class operand_compare. That allows use to share and unify
a lot of code. Apart from that I would like to learn current operand_equal_p
to handle new tree types.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests. I also
built Firefox, Godot engine.

Thanks,
Martin

Martin Liska (9):
  Replace int with boolean in predicate functions.
  operand_equal_p: add support for FIELD_DECL
  operand_equal_p: add support for OBJ_TYPE_REF.
  Strengthen alias_ptr_types_compatible_p in LTO mode.
  Come up with an abstraction.
  Integrate that for IPA ICF.
  IPA ICF: remove dead code
  Remove comparison for polymorphic types.
  Remove alias set comparison.

 gcc/alias.c                                   |   7 +-
 gcc/fold-const.c                              | 547 +++++++++++++++---
 gcc/fold-const.h                              |  30 +-
 gcc/ipa-icf-gimple.c                          | 328 ++---------
 gcc/ipa-icf-gimple.h                          |  16 +-
 gcc/ipa-icf.c                                 |  19 +-
 gcc/ipa-icf.h                                 |   3 -
 .../c-c++-common/Wstringop-truncation-4.c     |   2 +-
 gcc/testsuite/gcc.dg/ipa/ipa-icf-40.c         |  32 +
 gcc/testsuite/gcc.dg/{torture => }/pr70740.c  |   3 +-
 gcc/testsuite/gcc.dg/tree-ssa/pr64910-2.c     |   2 +-
 gcc/testsuite/gcc.dg/tree-ssa/pr79352.c       |   2 +-
 gcc/testsuite/gcc.dg/vect/vect-35-big-array.c |   3 +-
 gcc/testsuite/gcc.dg/vect/vect-35.c           |   3 +-
 gcc/testsuite/gfortran.dg/vect/vect-8.f90     |   2 +-
 gcc/tree.c                                    | 273 ---------
 16 files changed, 588 insertions(+), 684 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-40.c
 rename gcc/testsuite/gcc.dg/{torture => }/pr70740.c (77%)

--
2.22.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/9] Replace int with boolean in predicate functions.

Martin Liška-2

gcc/ChangeLog:

2019-07-24  Martin Liska  <[hidden email]>

        * fold-const.c (twoval_comparison_p): Replace int
        with bool as a return type.
        (simple_operand_p): Likewise.
        (operand_equal_p): Replace int with bool as a return type.
        * fold-const.h (operand_equal_p): Likewise.
---
 gcc/fold-const.c | 148 +++++++++++++++++++++++------------------------
 gcc/fold-const.h |   2 +-
 2 files changed, 75 insertions(+), 75 deletions(-)


0001-Replace-int-with-boolean-in-predicate-functions.patch (18K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH 4/9] Strengthen alias_ptr_types_compatible_p in LTO mode.

Martin Liška-2
In reply to this post by Martin Liška-2

gcc/ChangeLog:

2019-07-24  Martin Liska  <[hidden email]>

        * alias.c (alias_ptr_types_compatible_p): Strengten
        type comparison in LTO mode.
---
 gcc/alias.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)


0004-Strengthen-alias_ptr_types_compatible_p-in-LTO-mode.patch (569 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/9] operand_equal_p: add support for FIELD_DECL

Martin Liška-2
In reply to this post by Martin Liška-2

gcc/ChangeLog:

2019-07-24  Martin Liska  <[hidden email]>

        * fold-const.c (operand_equal_p): Support FIELD_DECL
        as well.
        * tree.c (add_expr): Hast DECL_FIELD_OFFSET and
        DECL_FIELD_BIT_OFFSET for FIELD_DECL.

gcc/testsuite/ChangeLog:

2019-07-24  Martin Liska  <[hidden email]>

        * gcc.dg/vect/vect-35-big-array.c: Vectorize one more loop.
        * gcc.dg/vect/vect-35.c: Likewise.
        * gcc.dg/pr70740.c: Move from torture and set -O2.
        * gfortran.dg/vect/vect-8.f90: Update scanned pattern.
---
 gcc/fold-const.c                              | 34 ++++++++++++++++---
 gcc/testsuite/gcc.dg/{torture => }/pr70740.c  |  3 +-
 gcc/testsuite/gcc.dg/vect/vect-35-big-array.c |  3 +-
 gcc/testsuite/gcc.dg/vect/vect-35.c           |  3 +-
 gcc/testsuite/gfortran.dg/vect/vect-8.f90     |  2 +-
 gcc/tree.c                                    |  4 +++
 6 files changed, 38 insertions(+), 11 deletions(-)
 rename gcc/testsuite/gcc.dg/{torture => }/pr70740.c (77%)


0002-operand_equal_p-add-support-for-FIELD_DECL.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/9] operand_equal_p: add support for OBJ_TYPE_REF.

Martin Liška-2
In reply to this post by Martin Liška-2

gcc/ChangeLog:

2019-07-24  Martin Liska  <[hidden email]>

        * fold-const.c (operand_equal_p): Support OBJ_TYPE_REF.
        * tree.c (add_expr): Hash parts of OBJ_TYPE_REF.
---
 gcc/fold-const.c | 21 +++++++++++++++++++++
 gcc/tree.c       |  9 +++++++++
 2 files changed, 30 insertions(+)


0003-operand_equal_p-add-support-for-OBJ_TYPE_REF.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH 5/9] Come up with an abstraction.

Martin Liška-2
In reply to this post by Martin Liška-2

gcc/ChangeLog:

2019-07-24  Martin Liska  <[hidden email]>

        * fold-const.c (operand_equal_p): Rename to ...
        (operand_compare::operand_equal_p): ... this.
        (add_expr):  Rename to ...
        (operand_compare::hash_operand): ... this.
        (operand_compare::operand_equal_valueize): Likewise.
        (operand_compare::hash_operand_valueize): Likewise.
        * fold-const.h (operand_equal_p): Set default
        value for last argument.
        (class operand_compare): New.
        * tree.c (add_expr): Move content to hash_operand.
---
 gcc/fold-const.c | 346 ++++++++++++++++++++++++++++++++++++++++++++++-
 gcc/fold-const.h |  30 +++-
 gcc/tree.c       | 286 ---------------------------------------
 3 files changed, 372 insertions(+), 290 deletions(-)


0005-Come-up-with-an-abstraction.patch (23K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH 6/9] Integrate that for IPA ICF.

Martin Liška-2
In reply to this post by Martin Liška-2

gcc/ChangeLog:

2019-07-24  Martin Liska  <[hidden email]>

        * ipa-icf-gimple.c (func_checker::hash_operand_valueize): New
        function created from compare_operand.
        (func_checker::compare_cst_or_decl): Remove handling of
        FIELD_DECLs as it's handled in operand_equal_p.
        (func_checker::compare_operand): Transform
        to func_checker::operand_equal_valueize.
        (func_checker::operand_equal_valueize): New.
        * ipa-icf-gimple.h (class func_checker): Inherit from
        operand_compare.
        * ipa-icf.c (sem_function::equals_private): Properly
        set push_cfun and pop_cfun.
---
 gcc/ipa-icf-gimple.c | 226 +++++++++++++------------------------------
 gcc/ipa-icf-gimple.h |   8 +-
 gcc/ipa-icf.c        |   7 +-
 3 files changed, 81 insertions(+), 160 deletions(-)


0006-Integrate-that-for-IPA-ICF.patch (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH 8/9] Remove comparison for polymorphic types.

Martin Liška-2
In reply to this post by Martin Liška-2

gcc/ChangeLog:

2019-07-24  Martin Liska  <[hidden email]>

        * ipa-icf-gimple.c (func_checker::func_checker): Do not
        initialize m_compare_polymorphic.
        (func_checker::compare_decl): Do not compare polymorphic types.
        * ipa-icf-gimple.h (m_compare_polymorphic): Remove.
        * ipa-icf.c (sem_function::equals_private): Do not call
        compare_polymorphic_p.
---
 gcc/ipa-icf-gimple.c | 18 ------------------
 gcc/ipa-icf-gimple.h |  4 ----
 gcc/ipa-icf.c        |  1 -
 3 files changed, 23 deletions(-)


0008-Remove-comparison-for-polymorphic-types.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH 9/9] Remove alias set comparison.

Martin Liška-2
In reply to this post by Martin Liška-2

gcc/ChangeLog:

2019-07-24  Martin Liska  <[hidden email]>

        * ipa-icf-gimple.c (func_checker::compatible_types_p):
        Do not compare alias sets.  It's handled by operand_equal_p.

gcc/testsuite/ChangeLog:

2019-07-24  Martin Liska  <[hidden email]>

        * c-c++-common/Wstringop-truncation-4.c: Disable IPA ICF.
        * gcc.dg/tree-ssa/pr64910-2.c: Likewise.
        * gcc.dg/tree-ssa/pr79352.c: Likewise.
        * gcc.dg/ipa/ipa-icf-40.c: New test.
---
 gcc/ipa-icf-gimple.c                          | 12 -------
 .../c-c++-common/Wstringop-truncation-4.c     |  2 +-
 gcc/testsuite/gcc.dg/ipa/ipa-icf-40.c         | 32 +++++++++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/pr64910-2.c     |  2 +-
 gcc/testsuite/gcc.dg/tree-ssa/pr79352.c       |  2 +-
 5 files changed, 35 insertions(+), 15 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-40.c


0009-Remove-alias-set-comparison.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH 7/9] IPA ICF: remove dead code

Martin Liška-2
In reply to this post by Martin Liška-2

gcc/ChangeLog:

2019-07-24  Martin Liska  <[hidden email]>

        * ipa-icf-gimple.c (func_checker::compare_ssa_name): Call
        compare_operand.
        (func_checker::compare_memory_operand): Remove.
        (func_checker::compare_cst_or_decl): Remove.
        (func_checker::operand_equal_valueize): Do not handle
        FIELD_DECL.
        (func_checker::compare_gimple_call): Call compare_operand.
        (func_checker::compare_gimple_assign): Likewise.
        * ipa-icf-gimple.h: Remove compare_cst_or_decl.
        * ipa-icf.c (sem_function::icf_handled_component_p): Remove.
        * ipa-icf.h (icf_handled_component_p): Remove.
---
 gcc/ipa-icf-gimple.c | 150 ++-----------------------------------------
 gcc/ipa-icf-gimple.h |   4 --
 gcc/ipa-icf.c        |  11 ----
 gcc/ipa-icf.h        |   3 -
 4 files changed, 6 insertions(+), 162 deletions(-)


0007-IPA-ICF-remove-dead-code.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/9] Strengthen alias_ptr_types_compatible_p in LTO mode.

Richard Biener-2
In reply to this post by Martin Liška-2
On Tue, Aug 6, 2019 at 5:43 PM Martin Liska <[hidden email]> wrote:

This warrants a comment like

  /* This function originally abstracts from simply comparing
get_deref_alias_set
     so that we are sure this still computes the same result after LTO
type merging
     is applied.  When in LTO type merging is done we can actually do
this compare.  */
  if (in_lto_p)
    return get_deref_alias_set (t1) == get_deref_alias_set (t2);
...

also note you want to call get_deref_alias_set as mentioned in the
function comment.

OK with this change.

Thanks,
Richard.

> gcc/ChangeLog:
>
> 2019-07-24  Martin Liska  <[hidden email]>
>
>         * alias.c (alias_ptr_types_compatible_p): Strengten
>         type comparison in LTO mode.
> ---
>  gcc/alias.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/9] operand_equal_p: add support for OBJ_TYPE_REF.

Richard Biener-2
In reply to this post by Martin Liška-2
On Tue, Aug 6, 2019 at 5:43 PM Martin Liska <[hidden email]> wrote:
>
>
> gcc/ChangeLog:

+       /* Virtual table call.  */
+       case OBJ_TYPE_REF:
+         {
+           if (!operand_equal_p (OBJ_TYPE_REF_EXPR (arg0),
+                                 OBJ_TYPE_REF_EXPR (arg1), flags))
+             return false;
+           if (virtual_method_call_p (arg0))
+             {
+               if (tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg0))
+                   != tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg1)))
+                 return false;
+               if (!types_same_for_odr (obj_type_ref_class (arg0),
+                                        obj_type_ref_class (arg1)))
+                 return false;
+               if (!operand_equal_p (OBJ_TYPE_REF_OBJECT (arg0),
+                                     OBJ_TYPE_REF_OBJECT (arg1), flags))
+                 return false;

this all gets deep into the devirt machinery, including looking at
ODR type hashes.  So I'm not sure if we really want to handle
it this "optimistic" in operand_equal_p and completely ignore
other operands when !virtual_method_call_p?  That is, why
not compare OBJ_TYPE_REF_TOKEN/OBJECT always at least?

Do we then have cases where the OBJ_TYPE_REF is actually
distinct according to the remaining check?

+             }


> 2019-07-24  Martin Liska  <[hidden email]>
>
>         * fold-const.c (operand_equal_p): Support OBJ_TYPE_REF.
>         * tree.c (add_expr): Hash parts of OBJ_TYPE_REF.
> ---
>  gcc/fold-const.c | 21 +++++++++++++++++++++
>  gcc/tree.c       |  9 +++++++++
>  2 files changed, 30 insertions(+)
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/9] operand_equal_p: add support for FIELD_DECL

Richard Biener-2
In reply to this post by Martin Liška-2
On Tue, Aug 6, 2019 at 5:44 PM Martin Liska <[hidden email]> wrote:
>
>
> gcc/ChangeLog:

So I suppose this isn't to call operand_equal_p on two FIELD_DECLs
but to make two COMPONENT_REFs "more equal"?  If so I then
I suggest to make this change "local" to the COMPONENT_REF handling.
This also interacts with path-based disambiguation so you want to make
sure to only make things equal here iff it wouldn't change the outcome
of path-based analysis.  Honza?

Richard.

> 2019-07-24  Martin Liska  <[hidden email]>
>
>         * fold-const.c (operand_equal_p): Support FIELD_DECL
>         as well.
>         * tree.c (add_expr): Hast DECL_FIELD_OFFSET and
>         DECL_FIELD_BIT_OFFSET for FIELD_DECL.
>
> gcc/testsuite/ChangeLog:
>
> 2019-07-24  Martin Liska  <[hidden email]>
>
>         * gcc.dg/vect/vect-35-big-array.c: Vectorize one more loop.
>         * gcc.dg/vect/vect-35.c: Likewise.
>         * gcc.dg/pr70740.c: Move from torture and set -O2.
>         * gfortran.dg/vect/vect-8.f90: Update scanned pattern.
> ---
>  gcc/fold-const.c                              | 34 ++++++++++++++++---
>  gcc/testsuite/gcc.dg/{torture => }/pr70740.c  |  3 +-
>  gcc/testsuite/gcc.dg/vect/vect-35-big-array.c |  3 +-
>  gcc/testsuite/gcc.dg/vect/vect-35.c           |  3 +-
>  gcc/testsuite/gfortran.dg/vect/vect-8.f90     |  2 +-
>  gcc/tree.c                                    |  4 +++
>  6 files changed, 38 insertions(+), 11 deletions(-)
>  rename gcc/testsuite/gcc.dg/{torture => }/pr70740.c (77%)
>
123