From patchwork Fri Dec 17 06:33:48 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Lance Taylor X-Patchwork-Id: 75837 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 9F5991007D3 for ; Fri, 17 Dec 2010 17:34:11 +1100 (EST) Received: (qmail 29883 invoked by alias); 17 Dec 2010 06:34:07 -0000 Received: (qmail 29686 invoked by uid 22791); 17 Dec 2010 06:34:04 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, SPF_HELO_PASS, TW_CC, TW_CP, T_RP_MATCHES_RCVD, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 17 Dec 2010 06:33:58 +0000 Received: from kpbe12.cbf.corp.google.com (kpbe12.cbf.corp.google.com [172.25.105.76]) by smtp-out.google.com with ESMTP id oBH6XuEX021575 for ; Thu, 16 Dec 2010 22:33:56 -0800 Received: from iwn8 (iwn8.prod.google.com [10.241.68.72]) by kpbe12.cbf.corp.google.com with ESMTP id oBH6XtQS028838 (version=TLSv1/SSLv3 cipher=RC4-MD5 bits=128 verify=NOT) for ; Thu, 16 Dec 2010 22:33:55 -0800 Received: by iwn8 with SMTP id 8so392051iwn.34 for ; Thu, 16 Dec 2010 22:33:55 -0800 (PST) Received: by 10.42.241.5 with SMTP id lc5mr534557icb.0.1292567635002; Thu, 16 Dec 2010 22:33:55 -0800 (PST) Received: from coign.google.com (adsl-71-133-8-30.dsl.pltn13.pacbell.net [71.133.8.30]) by mx.google.com with ESMTPS id c4sm667791ict.7.2010.12.16.22.33.53 (version=TLSv1/SSLv3 cipher=RC4-MD5); Thu, 16 Dec 2010 22:33:54 -0800 (PST) From: Ian Lance Taylor To: gcc-patches@gcc.gnu.org, gofrontend-dev@googlegroups.com Subject: Go patch committed: Avoid splitting stack for append and copy Date: Thu, 16 Dec 2010 22:33:48 -0800 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 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(©_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 + +/* 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); +}