Patchwork Go patch committed: Avoid splitting stack for append and copy

login
register
mail settings
Submitter Ian Taylor
Date Dec. 17, 2010, 6:33 a.m.
Message ID <mcrlj3ox60j.fsf@google.com>
Download mbox | patch
Permalink /patch/75837/
State New
Headers show

Comments

Ian Taylor - Dec. 17, 2010, 6:33 a.m.
In gccgo it's relatively expensive to split the stack, because it
requires two system calls: to disable signals and then to enable them
again.  Before this patch calls to the frequently used copy and append
predefined functions would always split the stack, because they called
memcpy and memmove.  This patch uses the no_stack_split attribute to
avoid splitting the stack, on the theory that memcpy and memmove are
unlikely to require much stack space and thus should be able to run in
the slop area which is always available.  I also simplified the calling
sequence for append slightly, passing in the size of the element rather
than a type descriptor, and not passing in the capacity of the second
argument which was not used.

Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.

Ian

Patch

diff -r 6326d4b106da go/expressions.cc
--- a/go/expressions.cc	Wed Dec 15 20:45:48 2010 -0800
+++ b/go/expressions.cc	Thu Dec 16 22:25:07 2010 -0800
@@ -7785,9 +7785,23 @@ 
 				    bytecount, element_size);
 	bytecount = fold_convert_loc(location, size_type_node, bytecount);
 
-	tree call = build_call_expr_loc(location,
-					built_in_decls[BUILT_IN_MEMMOVE],
-					3, arg1_val, arg2_val, bytecount);
+	arg1_val = fold_convert_loc(location, ptr_type_node, arg1_val);
+	arg2_val = fold_convert_loc(location, ptr_type_node, arg2_val);
+
+	static tree copy_fndecl;
+	tree call = Gogo::call_builtin(&copy_fndecl,
+				       location,
+				       "__go_copy",
+				       3,
+				       void_type_node,
+				       ptr_type_node,
+				       arg1_val,
+				       ptr_type_node,
+				       arg2_val,
+				       size_type_node,
+				       bytecount);
+	if (call == error_mark_node)
+	  return error_mark_node;
 
 	return fold_build2_loc(location, COMPOUND_EXPR, TREE_TYPE(len),
 			       call, len);
@@ -7800,12 +7814,29 @@ 
 	Expression* arg1 = args->front();
 	Expression* arg2 = args->back();
 
+	Array_type* at = arg1->type()->array_type();
+	Type* element_type = at->element_type();
+
 	tree arg1_tree = arg1->get_tree(context);
 	tree arg2_tree = arg2->get_tree(context);
 	if (arg1_tree == error_mark_node || arg2_tree == error_mark_node)
 	  return error_mark_node;
 
-	tree descriptor_tree = arg1->type()->type_descriptor_pointer(gogo);
+	Array_type* at2 = arg2->type()->array_type();
+	arg2_tree = save_expr(arg2_tree);
+	tree arg2_val = at2->value_pointer_tree(gogo, arg2_tree);
+	tree arg2_len = at2->length_tree(gogo, arg2_tree);
+	if (arg2_val == error_mark_node || arg2_len == error_mark_node)
+	  return error_mark_node;
+	arg2_val = fold_convert_loc(location, ptr_type_node, arg2_val);
+	arg2_len = fold_convert_loc(location, size_type_node, arg2_len);
+
+	tree element_type_tree = element_type->get_tree(gogo);
+	if (element_type_tree == error_mark_node)
+	  return error_mark_node;
+	tree element_size = TYPE_SIZE_UNIT(element_type_tree);
+	element_size = fold_convert_loc(location, size_type_node,
+					element_size);
 
 	// We rebuild the decl each time since the slice types may
 	// change.
@@ -7813,14 +7844,16 @@ 
 	return Gogo::call_builtin(&append_fndecl,
 				  location,
 				  "__go_append",
-				  3,
+				  4,
 				  TREE_TYPE(arg1_tree),
-				  TREE_TYPE(descriptor_tree),
-				  descriptor_tree,
 				  TREE_TYPE(arg1_tree),
 				  arg1_tree,
-				  TREE_TYPE(arg2_tree),
-				  arg2_tree);
+				  ptr_type_node,
+				  arg2_val,
+				  size_type_node,
+				  arg2_len,
+				  size_type_node,
+				  element_size);
       }
 
     case BUILTIN_REAL:
diff -r 6326d4b106da libgo/Makefile.am
--- a/libgo/Makefile.am	Wed Dec 15 20:45:48 2010 -0800
+++ b/libgo/Makefile.am	Thu Dec 16 22:25:07 2010 -0800
@@ -319,6 +319,7 @@ 
 	runtime/go-closed.c \
 	runtime/go-construct-map.c \
 	runtime/go-convert-interface.c \
+	runtime/go-copy.c \
 	runtime/go-defer.c \
 	runtime/go-deferred-recover.c \
 	runtime/go-eface-compare.c \
diff -r 6326d4b106da libgo/runtime/go-append.c
--- a/libgo/runtime/go-append.c	Wed Dec 15 20:45:48 2010 -0800
+++ b/libgo/runtime/go-append.c	Thu Dec 16 22:25:07 2010 -0800
@@ -4,37 +4,43 @@ 
    Use of this source code is governed by a BSD-style
    license that can be found in the LICENSE file.  */
 
-#include "go-assert.h"
 #include "go-type.h"
+#include "go-panic.h"
 #include "array.h"
 #include "runtime.h"
 #include "malloc.h"
 
+/* We should be OK if we don't split the stack here, since the only
+   libc functions we call are memcpy and memmove.  If we don't do
+   this, we will always split the stack, because of memcpy and
+   memmove.  */
+extern struct __go_open_array
+__go_append (struct __go_open_array, void *, size_t, size_t)
+  __attribute__ ((no_split_stack));
+
 struct __go_open_array
-__go_append (const struct __go_slice_type *type,
-	     struct __go_open_array a, struct __go_open_array b)
+__go_append (struct __go_open_array a, void *bvalues, size_t bcount,
+	     size_t element_size)
 {
-  size_t element_size;
-  unsigned int ucount;
+  size_t ucount;
   int count;
 
-  if (b.__values == NULL || b.__count == 0)
+  if (bvalues == NULL || bcount == 0)
     return a;
 
-  __go_assert (type->__common.__code == GO_SLICE);
-  element_size = type->__element_type->__size;
+  ucount = (size_t) a.__count + bcount;
+  count = (int) ucount;
+  if ((size_t) count != ucount || count <= a.__count)
+    __go_panic_msg ("append: slice overflow");
 
-  ucount = (unsigned int) a.__count + (unsigned int) b.__count;
-  count = (int) ucount;
-  __go_assert (ucount == (unsigned int) count && count >= a.__count);
   if (count > a.__capacity)
     {
       int m;
-      struct __go_open_array n;
+      void *n;
 
       m = a.__capacity;
       if (m == 0)
-	m = b.__count;
+	m = (int) bcount;
       else
 	{
 	  do
@@ -47,16 +53,15 @@ 
 	  while (m < count);
 	}
 
-      n.__values = __go_alloc (m * element_size);
-      n.__count = a.__count;
-      n.__capacity = m;
-      __builtin_memcpy (n.__values, a.__values, n.__count * element_size);
+      n = __go_alloc (m * element_size);
+      __builtin_memcpy (n, a.__values, a.__count * element_size);
 
-      a = n;
+      a.__values = n;
+      a.__capacity = m;
     }
 
   __builtin_memmove ((char *) a.__values + a.__count * element_size,
-		     b.__values, b.__count * element_size);
+		     bvalues, bcount * element_size);
   a.__count = count;
   return a;
 }
diff -r 6326d4b106da libgo/runtime/go-copy.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/libgo/runtime/go-copy.c	Thu Dec 16 22:25:07 2010 -0800
@@ -0,0 +1,21 @@ 
+/* go-append.c -- the go builtin copy function.
+
+   Copyright 2010 The Go Authors. All rights reserved.
+   Use of this source code is governed by a BSD-style
+   license that can be found in the LICENSE file.  */
+
+#include <stddef.h>
+
+/* We should be OK if we don't split the stack here, since we are just
+   calling memmove which shouldn't need much stack.  If we don't do
+   this we will always split the stack, because of memmove.  */
+
+extern void
+__go_copy (void *, void *, size_t)
+  __attribute__ ((no_split_stack));
+
+void
+__go_copy (void *a, void *b, size_t len)
+{
+  __builtin_memmove (a, b, len);
+}