Patchwork ObjC: avoid creating temporary nodes just to compare method signatures

login
register
mail settings
Submitter Nicola Pero
Date April 12, 2011, 6:04 p.m.
Message ID <1302631488.416928488@192.168.4.58>
Download mbox | patch
Permalink /patch/90842/
State New
Headers show

Comments

Nicola Pero - April 12, 2011, 6:04 p.m.
This patch fixes another inefficiency in the Objective-C compiler.

When it was comparing method signatures, it would copy all the argument types
into some temporary node chains, and then compare these copies.  The copies
are then thrown away.  It all seems really pointless (since you can just
access the argument types and compare them directly, as this patch does),
but the inefficiency was hidden by the usage of get_arg_type_list() which
looked like a very innocent and professionally looking API, while it does
a very expensive copy of everything into a newly allocated chain of nodes.

This patch saves about 10k node allocations when compiling a file that
includes the Objective-C GNUstep system frameworks.  With -fsyntax-only,
that gives about a 0.5% speedup.

Ok to commit ?

Thanks
Mike Stump - April 12, 2011, 6:39 p.m.
On Apr 12, 2011, at 11:04 AM, Nicola Pero wrote:
> This patch fixes another inefficiency in the Objective-C compiler.

> Ok to commit ?

Ok.

Patch

Index: ChangeLog
===================================================================
--- ChangeLog   (revision 172328)
+++ ChangeLog   (working copy)
@@ -1,5 +1,12 @@ 
 2011-04-12  Nicola Pero  <nicola.pero@meta-innovation.com>
 
+       * objc-act.c (comp_proto_with_proto): Do not create and use
+       inefficient temporary argument lists.  Compare the arguments
+       directly.  (match_proto_with_proto): Removed; incorporated into
+       comp_proto_with_proto ().
+       
+2011-04-12  Nicola Pero  <nicola.pero@meta-innovation.com>
+
        * objc-act.c (printable_ivar_name): New.
        (add_instance_variable): Call printable_ivar_name() when an error
        message needs to be printed.  Do not prepare the instance variable
Index: objc-act.c
===================================================================
--- objc-act.c  (revision 172328)
+++ objc-act.c  (working copy)
@@ -147,7 +147,6 @@  static void objc_gen_property_data (tree, tree);
 static void objc_synthesize_getter (tree, tree, tree);
 static void objc_synthesize_setter (tree, tree, tree);
 static char *objc_build_property_setter_name (tree);
-static int match_proto_with_proto (tree, tree, int);
 static tree lookup_property (tree, tree);
 static tree lookup_property_in_list (tree, tree);
 static tree lookup_property_in_protocol_list (tree, tree);
@@ -4836,13 +4835,13 @@  objc_method_decl (enum tree_code opcode)
   return opcode == INSTANCE_METHOD_DECL || opcode == CLASS_METHOD_DECL;
 }
 
-/* Used by `build_objc_method_call' and `comp_proto_with_proto'.  Return
-   an argument list for method METH.  CONTEXT is either METHOD_DEF or
-   METHOD_REF, saying whether we are trying to define a method or call
-   one.  SUPERFLAG says this is for a send to super; this makes a
-   difference for the NeXT calling sequence in which the lookup and
-   the method call are done together.  If METH is null, user-defined
-   arguments (i.e., beyond self and _cmd) shall be represented by `...'.  */
+/* Used by `build_objc_method_call'.  Return an argument list for
+   method METH.  CONTEXT is either METHOD_DEF or METHOD_REF, saying
+   whether we are trying to define a method or call one.  SUPERFLAG
+   says this is for a send to super; this makes a difference for the
+   NeXT calling sequence in which the lookup and the method call are
+   done together.  If METH is null, user-defined arguments (i.e.,
+   beyond self and _cmd) shall be represented by `...'.  */
 
 tree
 get_arg_type_list (tree meth, int context, int superflag)
@@ -8241,19 +8240,13 @@  objc_types_share_size_and_alignment (tree type1, t
 static int
 comp_proto_with_proto (tree proto1, tree proto2, int strict)
 {
+  tree type1, type2;
+
   /* The following test is needed in case there are hashing
      collisions.  */
   if (METHOD_SEL_NAME (proto1) != METHOD_SEL_NAME (proto2))
     return 0;
 
-  return match_proto_with_proto (proto1, proto2, strict);
-}
-
-static int
-match_proto_with_proto (tree proto1, tree proto2, int strict)
-{
-  tree type1, type2;
-
   /* Compare return types.  */
   type1 = TREE_VALUE (TREE_TYPE (proto1));
   type2 = TREE_VALUE (TREE_TYPE (proto2));
@@ -8263,19 +8256,75 @@  comp_proto_with_proto (tree proto1, tree proto2, i
     return 0;
 
   /* Compare argument types.  */
-  for (type1 = get_arg_type_list (proto1, METHOD_REF, 0),
-       type2 = get_arg_type_list (proto2, METHOD_REF, 0);
-       type1 && type2;
-       type1 = TREE_CHAIN (type1), type2 = TREE_CHAIN (type2))
-    {
-      if (!objc_types_are_equivalent (TREE_VALUE (type1), TREE_VALUE (type2))
-         && (strict
-             || !objc_types_share_size_and_alignment (TREE_VALUE (type1),
-                                                      TREE_VALUE (type2))))
-       return 0;
-    }
 
-  return (!type1 && !type2);
+  /* The first argument (objc_object_type) is always the same, no need
+     to compare.  */
+
+  /* The second argument (objc_selector_type) is always the same, no
+     need to compare.  */
+
+  /* Compare the other arguments.  */
+  {
+    tree arg1, arg2;
+
+    /* Compare METHOD_SEL_ARGS.  */
+    for (arg1 = METHOD_SEL_ARGS (proto1), arg2 = METHOD_SEL_ARGS (proto2);
+        arg1 && arg2;
+        arg1 = DECL_CHAIN (arg1), arg2 = DECL_CHAIN (arg2))
+      {
+       type1 = TREE_VALUE (TREE_TYPE (arg1));
+       type2 = TREE_VALUE (TREE_TYPE (arg2));
+       
+       /* FIXME: Do we need to decay argument types to compare them ?  */
+       type1 = objc_decay_parm_type (type1);
+       type2 = objc_decay_parm_type (type2);
+       
+       if (!objc_types_are_equivalent (type1, type2)
+           && (strict || !objc_types_share_size_and_alignment (type1, type2)))
+         return 0;
+      }
+    
+    /* The loop ends when arg1 or arg2 are NULL.  Make sure they are
+       both NULL.  */
+    if (arg1 != arg2)
+      return 0;
+
+    /* Compare METHOD_ADD_ARGS.  */
+    if ((METHOD_ADD_ARGS (proto1) && !METHOD_ADD_ARGS (proto2))
+       || (METHOD_ADD_ARGS (proto2) && !METHOD_ADD_ARGS (proto1)))
+      return 0;
+
+    if (METHOD_ADD_ARGS (proto1))
+      {
+       for (arg1 = TREE_CHAIN (METHOD_ADD_ARGS (proto1)), arg2 = TREE_CHAIN (METHOD_ADD_ARGS (proto2));
+            arg1 && arg2;
+            arg1 = TREE_CHAIN (arg1), arg2 = TREE_CHAIN (arg2))
+         {
+           type1 = TREE_TYPE (TREE_VALUE (arg1));
+           type2 = TREE_TYPE (TREE_VALUE (arg2));
+           
+           /* FIXME: Do we need to decay argument types to compare them ?  */
+           type1 = objc_decay_parm_type (type1);
+           type2 = objc_decay_parm_type (type2);
+           
+           if (!objc_types_are_equivalent (type1, type2)
+               && (strict || !objc_types_share_size_and_alignment (type1, type2)))
+             return 0;
+         }
+      }
+    
+    /* The loop ends when arg1 or arg2 are NULL.  Make sure they are
+       both NULL.  */
+    if (arg1 != arg2)
+      return 0;
+
+    /* Compare METHOD_ADD_ARGS_ELLIPSIS_P.  */
+    if (METHOD_ADD_ARGS_ELLIPSIS_P (proto1) != METHOD_ADD_ARGS_ELLIPSIS_P (proto2))
+      return 0;
+  }
+
+  /* Success.  */
+  return 1;
 }
 
 /* This routine returns true if TYPE is a valid objc object type,