From patchwork Sun Aug 1 10:28:40 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: 60446 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 495E9B70CE for ; Sun, 1 Aug 2010 20:28:58 +1000 (EST) Received: (qmail 24625 invoked by alias); 1 Aug 2010 10:28:56 -0000 Received: (qmail 24614 invoked by uid 22791); 1 Aug 2010 10:28:55 -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_TM, 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; Sun, 01 Aug 2010 10:28:48 +0000 Received: from kpbe18.cbf.corp.google.com (kpbe18.cbf.corp.google.com [172.25.105.82]) by smtp-out.google.com with ESMTP id o71ASk1a024358 for ; Sun, 1 Aug 2010 03:28:46 -0700 Received: from ewy24 (ewy24.prod.google.com [10.241.103.24]) by kpbe18.cbf.corp.google.com with ESMTP id o71ASiYk028436 for ; Sun, 1 Aug 2010 03:28:45 -0700 Received: by ewy24 with SMTP id 24so1370414ewy.37 for ; Sun, 01 Aug 2010 03:28:44 -0700 (PDT) Received: by 10.213.22.201 with SMTP id o9mr2775958ebb.71.1280658524424; Sun, 01 Aug 2010 03:28:44 -0700 (PDT) Received: from coign.google.com (dhcp-172-28-249-170.lul.corp.google.com [172.28.249.170]) by mx.google.com with ESMTPS id u9sm759852eeh.11.2010.08.01.03.28.42 (version=TLSv1/SSLv3 cipher=RC4-MD5); Sun, 01 Aug 2010 03:28:42 -0700 (PDT) From: Ian Lance Taylor To: gcc-patches@gcc.gnu.org, gofrontend-dev@googlegroups.com Subject: [gccgo] Register all global variables explicitly Date: Sun, 01 Aug 2010 03:28:40 -0700 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 This patch changes libgo to use an explicit list of global variables defined in Go which may contain pointers, rather than scanning the entire data section of the executable. Each package registers its own list of variables. This has the advantage of only scanning variables for imported packages in libgo, rather than all packages. This also removes the changes to crtstuff.c I added to support finding the data section at runtime. Committed to gccgo branch. Ian diff -r db5d6acb8d42 go/gogo-tree.cc --- a/go/gogo-tree.cc Sun Aug 01 03:18:13 2010 -0700 +++ b/go/gogo-tree.cc Sun Aug 01 03:20:48 2010 -0700 @@ -215,6 +215,131 @@ } } +// Register global variables with the garbage collector. We need to +// register all variables which can hold a pointer value. They become +// roots during the mark phase. We build a struct that is easy to +// hook into a list of roots. + +// struct __go_gc_root_list +// { +// struct __go_gc_root_list* __next; +// struct __go_gc_root +// { +// void* __decl; +// size_t __size; +// } __roots[]; +// }; + +// The last entry in the roots array has a NULL decl field. + +void +Gogo::register_gc_vars(const std::vector& var_gc, + tree* init_stmt_list) +{ + if (var_gc.empty()) + return; + + size_t count = var_gc.size(); + + tree root_type = Gogo::builtin_struct(NULL, "__go_gc_root", NULL_TREE, 2, + "__next", + ptr_type_node, + "__size", + sizetype); + + tree index_type = build_index_type(size_int(count)); + tree array_type = build_array_type(root_type, index_type); + + tree root_list_type = make_node(RECORD_TYPE); + root_list_type = Gogo::builtin_struct(NULL, "__go_gc_root_list", + root_list_type, 2, + "__next", + build_pointer_type(root_list_type), + "__roots", + array_type); + + // Build an initialier for the __roots array. + + VEC(constructor_elt,gc)* roots_init = VEC_alloc(constructor_elt, gc, + count + 1); + + size_t i = 0; + for (std::vector::const_iterator p = var_gc.begin(); + p != var_gc.end(); + ++p, ++i) + { + VEC(constructor_elt,gc)* init = VEC_alloc(constructor_elt, gc, 2); + + constructor_elt* elt = VEC_quick_push(constructor_elt, init, NULL); + tree field = TYPE_FIELDS(root_type); + elt->index = field; + tree decl = (*p)->get_tree(this, NULL); + gcc_assert(TREE_CODE(decl) == VAR_DECL); + elt->value = build_fold_addr_expr(decl); + + elt = VEC_quick_push(constructor_elt, init, NULL); + field = TREE_CHAIN(field); + elt->index = field; + elt->value = DECL_SIZE_UNIT(decl); + + elt = VEC_quick_push(constructor_elt, roots_init, NULL); + elt->index = size_int(i); + elt->value = build_constructor(root_type, init); + } + + // The list ends with a NULL entry. + + VEC(constructor_elt,gc)* init = VEC_alloc(constructor_elt, gc, 2); + + constructor_elt* elt = VEC_quick_push(constructor_elt, init, NULL); + tree field = TYPE_FIELDS(root_type); + elt->index = field; + elt->value = fold_convert(TREE_TYPE(field), null_pointer_node); + + elt = VEC_quick_push(constructor_elt, init, NULL); + field = TREE_CHAIN(field); + elt->index = field; + elt->value = size_zero_node; + + elt = VEC_quick_push(constructor_elt, roots_init, NULL); + elt->index = size_int(i); + elt->value = build_constructor(root_type, init); + + // Build a constructor for the struct. + + VEC(constructor_elt,gc*) root_list_init = VEC_alloc(constructor_elt, gc, 2); + + elt = VEC_quick_push(constructor_elt, root_list_init, NULL); + field = TYPE_FIELDS(root_list_type); + elt->index = field; + elt->value = fold_convert(TREE_TYPE(field), null_pointer_node); + + elt = VEC_quick_push(constructor_elt, root_list_init, NULL); + field = TREE_CHAIN(field); + elt->index = field; + elt->value = build_constructor(array_type, roots_init); + + // Build a decl to register. + + tree decl = build_decl(BUILTINS_LOCATION, VAR_DECL, + create_tmp_var_name("gc"), root_list_type); + DECL_EXTERNAL(decl) = 0; + TREE_PUBLIC(decl) = 0; + TREE_STATIC(decl) = 1; + DECL_ARTIFICIAL(decl) = 1; + DECL_INITIAL(decl) = build_constructor(root_list_type, root_list_init); + rest_of_decl_compilation(decl, 1, 0); + + static tree register_gc_fndecl; + tree call = Gogo::call_builtin(®ister_gc_fndecl, BUILTINS_LOCATION, + "__go_register_gc_roots", + 1, + void_type_node, + build_pointer_type(root_list_type), + build_fold_addr_expr(decl)); + append_to_statement_list(call, init_stmt_list); +} + // Build the decl for the initialization function. tree @@ -526,6 +651,12 @@ // A list of variable initializations. Var_inits var_inits; + // A list of variables which need to be registered with the garbage + // collector. + std::vector var_gc; + var_gc.reserve(count); + + tree var_init_stmt_list = NULL_TREE; size_t i = 0; for (Bindings::const_definitions_iterator p = bindings->begin_definitions(); p != bindings->end_definitions(); @@ -626,15 +757,25 @@ { if (no->var_value()->init() == NULL && !no->var_value()->has_pre_init()) - append_to_statement_list(var_init_tree, &init_stmt_list); + append_to_statement_list(var_init_tree, &var_init_stmt_list); else var_inits.push_back(Var_init(no, var_init_tree)); } + + if (!is_sink && no->var_value()->type()->has_pointer()) + var_gc.push_back(no); } } - // Initialize the variables, first sorting them into a workable - // order. + // Register global variables with the garbage collector. + this->register_gc_vars(var_gc, &init_stmt_list); + + // Simple variable initializations, after all variables are + // registered. + append_to_statement_list(var_init_stmt_list, &init_stmt_list); + + // Complex variable initializations, first sorting them into a + // workable order. if (!var_inits.empty()) { sort_var_inits(&var_inits); diff -r db5d6acb8d42 go/gogo.cc --- a/go/gogo.cc Sun Aug 01 03:18:13 2010 -0700 +++ b/go/gogo.cc Sun Aug 01 03:20:48 2010 -0700 @@ -1326,12 +1326,15 @@ else if ((*p)->is_const()) (*p)->const_value()->determine_type(); - // If this is a global variable which requires runtime - // initialization, we need an initialization function. We know - // that we will see all global variables here. - if ((*p)->is_variable()) + // See if a variable requires us to build an initialization + // function. We know that we will see all global variables + // here. + if (!this->need_init_fn_ && (*p)->is_variable()) { Variable* variable = (*p)->var_value(); + + // If this is a global variable which requires runtime + // initialization, we need an initialization function. if (!variable->is_global() || variable->init() == NULL) ; else if (variable->type()->interface_type() != NULL) @@ -1342,6 +1345,12 @@ this->need_init_fn_ = true; else if (variable->init()->is_nonconstant_composite_literal()) this->need_init_fn_ = true; + + // If this is a global variable which holds a pointer value, + // then we need an initialization function to register it as a + // GC root. + if (variable->is_global() && variable->type()->has_pointer()) + this->need_init_fn_ = true; } } diff -r db5d6acb8d42 go/gogo.h --- a/go/gogo.h Sun Aug 01 03:18:13 2010 -0700 +++ b/go/gogo.h Sun Aug 01 03:20:48 2010 -0700 @@ -605,6 +605,10 @@ void init_imports(tree*); + // Register variables with the garbage collector. + void + register_gc_vars(const std::vector&, tree*); + // Build a pointer to a Go string constant. This returns a pointer // to the pointer. tree diff -r db5d6acb8d42 libgo/runtime/malloc.h --- a/libgo/runtime/malloc.h Sun Aug 01 03:18:13 2010 -0700 +++ b/libgo/runtime/malloc.h Sun Aug 01 03:20:48 2010 -0700 @@ -375,6 +375,7 @@ void MProf_Malloc(void*, uintptr); void MProf_Free(void*, uintptr); +void MProf_Mark(void (*scan)(int32, byte *, int64)); // Malloc profiling settings. // Must match definition in extern.go. diff -r db5d6acb8d42 libgo/runtime/mfinal.c --- a/libgo/runtime/mfinal.c Sun Aug 01 03:18:13 2010 -0700 +++ b/libgo/runtime/mfinal.c Sun Aug 01 03:20:48 2010 -0700 @@ -167,11 +167,12 @@ } void -walkfintab(void (*fn)(void*)) +walkfintab(void (*fn)(void*), void (*scan)(int32, byte *, int64)) { void **key; void **ekey; + scan(0, (byte*)&fintab, sizeof fintab); lock(&finlock); key = fintab.key; ekey = key + fintab.max; diff -r db5d6acb8d42 libgo/runtime/mgc0.c --- a/libgo/runtime/mgc0.c Sun Aug 01 03:18:13 2010 -0700 +++ b/libgo/runtime/mgc0.c Sun Aug 01 03:20:48 2010 -0700 @@ -103,80 +103,50 @@ scanblock(1, v, size); } -struct globals { - byte *start; - uintptr size; +struct root_list { + struct root_list *next; + struct root { + void *decl; + size_t size; + } roots[]; }; -// FIXME: This needs to grow as needed. -#define GLOBALS_ENTRIES 16 +static struct root_list* roots; -static struct globals globals[GLOBALS_ENTRIES]; - -// Called by runtime. void -__go_register_mem(void *start, void *end) +__go_register_gc_roots (struct root_list* r) { - int i; - - if(start == nil || end == nil) - throw("__go_register_mem"); - if(start == end) - return; - for(i = 0; i < GLOBALS_ENTRIES; ++i) { - if(globals[i].start == nil) { - globals[i].start = (byte*)start; - globals[i].size = (byte*)end - (byte*)start; - return; - } - } - throw("__go_register_mem out of space"); -} - -// Called by runtime for dlclose. -void -__go_deregister_mem(void *start, void *end) -{ - int i; - - if(start == end) - return; - for(i = 0; i < GLOBALS_ENTRIES; ++i) { - if(globals[i].start == (byte*)start - && globals[i].size == (size_t)((byte*)end - (byte*)start)) { - globals[i].start = nil; - return; - } - } - throw("__go_deregister_mem not found"); + // FIXME: This needs locking if multiple goroutines can call + // dlopen simultaneously. + r->next = roots; + roots = r; } static void mark(void) { - int i; + struct root_list *pl; - // mark data+bss. - // skip mheap itself, which has no interesting pointers - // and is mostly zeroed and would not otherwise be paged in. - for(i = 0; i < GLOBALS_ENTRIES; ++i) { - if (globals[i].start == nil) - continue; - if ((byte*)&mheap >= globals[i].start - && (byte*)&mheap < globals[i].start + globals[i].size) { - scanblock(0, globals[i].start, (byte*)&mheap - globals[i].start); - scanblock(0, (byte*)(&mheap+1), - globals[i].start + globals[i].size - (byte*)(&mheap+1)); + for(pl = roots; pl != nil; pl = pl->next) { + struct root* pr = &pl->roots[0]; + while(1) { + void *decl = pr->decl; + if(decl == nil) + break; + scanblock(0, decl, pr->size); + pr++; } - else - scanblock(0, globals[i].start, globals[i].size); } + scanblock(0, (byte*)&m0, sizeof m0); + scanblock(0, (byte*)&finq, sizeof finq); + MProf_Mark(scanblock); + // mark stacks __go_scanstacks(scanblock); // mark things pointed at by objects with finalizers - walkfintab(markfin); + walkfintab(markfin, scanblock); } // free RefNone, free & queue finalizers for RefNone|RefHasFinalizer, reset RefSome diff -r db5d6acb8d42 libgo/runtime/mprof.goc --- a/libgo/runtime/mprof.goc Sun Aug 01 03:18:13 2010 -0700 +++ b/libgo/runtime/mprof.goc Sun Aug 01 03:20:48 2010 -0700 @@ -288,3 +288,12 @@ if(__sync_bool_compare_and_swap(&m->gcing, 1, 0)) __go_run_goroutine_gc(102); } + +void +MProf_Mark(void (*scan)(int32, byte *, int64)) +{ + // buckhash is not allocated via mallocgc. + scan(0, (byte*)&buckets, sizeof buckets); + scan(0, (byte*)&addrhash, sizeof addrhash); + scan(0, (byte*)&addrfree, sizeof addrfree); +} diff -r db5d6acb8d42 libgo/runtime/runtime.h --- a/libgo/runtime/runtime.h Sun Aug 01 03:18:13 2010 -0700 +++ b/libgo/runtime/runtime.h Sun Aug 01 03:20:48 2010 -0700 @@ -72,6 +72,8 @@ extern __thread M* m; +extern M m0; + #ifdef __rtems__ #undef __thread #endif @@ -164,7 +166,7 @@ void free(void *v); struct __go_func_type; void addfinalizer(void*, void(*fn)(void*), const struct __go_func_type *); -void walkfintab(void (*fn)(void*)); +void walkfintab(void (*fn)(void*), void (*scan)(int32, byte *, int64)); #define runtime_mmap mmap #define cas(pval, old, new) __sync_bool_compare_and_swap (pval, old, new) Index: gcc/crtstuff.c =================================================================== --- gcc/crtstuff.c (revision 162490) +++ gcc/crtstuff.c (working copy) @@ -116,10 +116,6 @@ call_ ## FUNC (void) \ # define HIDDEN_DTOR_LIST_END #endif -/* If the Go runtime is present, we currently always register memory - with it. */ -#define GO_REGISTER_MEM - /* We do not want to add the weak attribute to the declarations of these routines in unwind-dw2-fde.h because that will cause the definition of these symbols to be weak as well. @@ -156,12 +152,6 @@ extern void __do_global_ctors_1 (void); /* Likewise for _Jv_RegisterClasses. */ extern void _Jv_RegisterClasses (void *) TARGET_ATTRIBUTE_WEAK; -/* And for __go_register_mem. */ -extern char _end[] TARGET_ATTRIBUTE_WEAK - __attribute__ ((__visibility__ ("hidden"))); -extern void __go_register_mem (void *, void *) TARGET_ATTRIBUTE_WEAK; -extern void __go_deregister_mem (void *, void *) TARGET_ATTRIBUTE_WEAK; - #ifdef OBJECT_FORMAT_ELF /* Declare a pointer to void function type. */ @@ -338,11 +328,6 @@ __do_global_dtors_aux (void) #endif #endif -#ifdef GO_REGISTER_MEM - if (__go_deregister_mem) - __go_deregister_mem (__CTOR_LIST__, _end); -#endif - completed = 1; } @@ -362,7 +347,7 @@ __do_global_dtors_aux_1 (void) CRT_CALL_STATIC_FUNCTION (INIT_SECTION_ASM_OP, __do_global_dtors_aux_1) #endif -#if defined(USE_EH_FRAME_REGISTRY) || defined(JCR_SECTION_NAME) || defined(GO_REGISTER_MEM) +#if defined(USE_EH_FRAME_REGISTRY) || defined(JCR_SECTION_NAME) /* Stick a call to __register_frame_info into the .init section. For some reason calls with no arguments work more reliably in .init, so stick the call in another function. */ @@ -392,10 +377,6 @@ frame_dummy (void) register_classes (__JCR_LIST__); } #endif /* JCR_SECTION_NAME */ -#ifdef GO_REGISTER_MEM - if (__go_register_mem) - __go_register_mem (__CTOR_LIST__, _end); -#endif } #ifdef INIT_SECTION_ASM_OP @@ -466,14 +447,9 @@ __do_global_dtors (void) if (__deregister_frame_info) __deregister_frame_info (__EH_FRAME_BEGIN__); #endif - -#ifdef GO_REGISTER_MEM - if (__go_deregister_mem) - __go_deregister_mem (__CTOR_LIST__, _end); -#endif } -#if defined(USE_EH_FRAME_REGISTRY) || defined(JCR_SECTION_NAME) || defined(GO_REGISTER_MEM) +#if defined(USE_EH_FRAME_REGISTRY) || defined(JCR_SECTION_NAME) /* A helper function for __do_global_ctors, which is in crtend.o. Here in crtbegin.o, we can reference a couple of symbols not visible there. Plus, since we're before libgcc.a, we have no problems referencing @@ -495,10 +471,6 @@ __do_global_ctors_1(void) register_classes (__JCR_LIST__); } #endif -#ifdef GO_REGISTER_MEM - if (__go_register_mem) - __go_register_mem (__CTOR_LIST__, _end); -#endif } #endif /* USE_EH_FRAME_REGISTRY || JCR_SECTION_NAME */ @@ -644,7 +616,7 @@ void __do_global_ctors (void) { func_ptr *p; -#if defined(USE_EH_FRAME_REGISTRY) || defined(JCR_SECTION_NAME) || defined(GO_REGISTER_MEM) +#if defined(USE_EH_FRAME_REGISTRY) || defined(JCR_SECTION_NAME) __do_global_ctors_1(); #endif for (p = __CTOR_END__ - 1; *p != (func_ptr) -1; p--)