[m-rev.] for review: stack segments

[Peter Wang]

On 2006-10-31, Zoltan Somogyi <zs at csse.unimelb.edu.au> wrote:
> For review by anyone.
> 
> Zoltan.
> 
> Add a mechanism for growing the stacks on demand by adding new segments
> to them. You can ask for the new mechanism via a new grade component, stseg
> (short for "stack segments").

That was quick :-)

> Index: compiler/options.m
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/compiler/options.m,v
> retrieving revision 1.532
> diff -u -b -r1.532 options.m
> --- compiler/options.m	5 Oct 2006 04:59:21 -0000	1.532
> +++ compiler/options.m	27 Oct 2006 15:58:23 -0000
> @@ -1774,6 +1776,7 @@
>  long_option("type-layout",          type_layout).
>  long_option("maybe-thread-safe",    maybe_thread_safe_opt).
>  long_option("extend-stacks-when-needed",    extend_stacks_when_needed).
> +long_option("stack-segments",       stack_segments).
>  % Data representation options
>  long_option("reserve-tag",          reserve_tag).
>  long_option("use-minimal-model-stack_copy", use_minimal_model_stack_copy).
> @@ -3575,7 +3578,10 @@
>          "\tattribute.  The default is no.",
>          "--extend-stacks-when-needed",
>          "\tSpecify that code that increments a stack pointer must",
> -        "\textend the stack when this is needed."
> +        "\textend the stack when this is needed.",
> +        "--stack-segments-needed",

--stack-segments

> Index: runtime/mercury_memory_zones.h
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_memory_zones.h,v
> retrieving revision 1.17
> diff -u -b -r1.17 mercury_memory_zones.h
> --- runtime/mercury_memory_zones.h	13 Sep 2005 08:25:39 -0000	1.17
> +++ runtime/mercury_memory_zones.h	29 Oct 2006 21:11:45 -0000
> @@ -194,7 +202,7 @@
>  ** has the same behaviour as MR_create_zone, except instead of allocating
>  ** the memory, it takes a pointer to a region of memory that must be at
>  ** least Size + MR_unit[*] bytes, or if MR_PROTECTPAGE is defined, then it
> -** must be at least Size + 2 * unit[*] bytes.
> +** must be at least Size + 2 * MR_unit[*] bytes.
>  ** If it fails to protect the redzone then it exits.
>  ** If MR_CHECK_OVERFLOW_VIA_MPROTECT is unavailable, then the last two
>  ** arguments are ignored.
> @@ -252,6 +260,12 @@
>  extern	void		MR_debug_memory_zone(FILE *fp, MR_MemoryZone *zone);
>  
>  /*
> +** Return the given zone to tyhe list of free zones.

the

> +*/
> +
> +extern	void		MR_unget_zone(MR_MemoryZone *zone);


> Index: runtime/mercury_stack_trace.c
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_stack_trace.c,v
> retrieving revision 1.74
> diff -u -b -r1.74 mercury_stack_trace.c
> --- runtime/mercury_stack_trace.c	29 Sep 2006 06:34:55 -0000	1.74
> +++ runtime/mercury_stack_trace.c	30 Oct 2006 05:27:26 -0000
> @@ -315,6 +318,18 @@
>          return MR_STEP_OK;
>      }
>  
> +#ifndef MR_HIGHLEVEL_CODE
> +    if (success == MR_ENTRY(MR_pop_detstack_segment)) {
> +        success = (MR_Code *) MR_based_stackvar(*stack_trace_sp_ptr, 2);
> +        *stack_trace_sp_ptr = (MR_Word *)
> +            MR_based_stackvar(*stack_trace_sp_ptr, 1);
> +    } else if (success == MR_ENTRY(MR_pop_nondetstack_segment)) {
> +        success = MR_succip_slot(*stack_trace_curfr_ptr);
> +        *stack_trace_curfr_ptr = (MR_Word *)
> +            MR_based_framevar(*stack_trace_curfr_ptr, 1);
> +    }
> +#endif /* MR_HIGHLEVEL_CODE */
> +

Missing #ifdef MR_STACK_SEGMENTS around this?

> Index: runtime/mercury_stacks.c
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_stacks.c,v
> retrieving revision 1.17
> diff -u -b -r1.17 mercury_stacks.c
> --- runtime/mercury_stacks.c	25 Nov 2005 05:40:47 -0000	1.17
> +++ runtime/mercury_stacks.c	30 Oct 2006 06:02:27 -0000
> @@ -191,6 +197,255 @@
>  
>  /***************************************************************************/
>  
> +#ifndef  MR_HIGHLEVEL_CODE
> +
> +#ifdef  MR_STACK_SEGMENTS
> +
> +/*
> +** These three global variables are used in the MR_detstack_extend_and_check
> +** and MR_nondetstack_extend_and_check macros and nowhere else.
> +*/
> +
> +MR_Code *MR_post_extend_succip;
> +MR_Word *MR_pre_extend_maxfr;
> +MR_Word *MR_pre_extend_curfr;

These globals are going to be a problem for multithreaded code.

> +MR_declare_entry(MR_pop_detstack_segment);
> +MR_declare_entry(MR_pop_nondetstack_segment);
> +
> +MR_Word *MR_new_detstack_segment(MR_Word *sp, int n)
> +{
> +    MR_Word         *old_sp;
> +    MR_MemoryZones  *list;
> +    MR_MemoryZone   *new_zone;
> +
> +    old_sp = sp;
> +
> +    new_zone = MR_create_zone("detstack_segment", 0, MR_detstack_size, 0,
> +        MR_detstack_zone_size, MR_default_handler);
> +
> +    list = GC_MALLOC_UNCOLLECTABLE(sizeof(MR_MemoryZones));

MR_GC_NEW_UNCOLLECTABLE

> +MR_Word *
> +MR_new_nondetstack_segment(MR_Word *maxfr, int n)
> +{
> +    MR_Word         *old_maxfr;
> +    MR_MemoryZones  *list;
> +    MR_MemoryZone   *new_zone;
> +
> +    old_maxfr = maxfr;
> +
> +    new_zone = MR_create_zone("nondetstack_segment", 0, MR_nondetstack_size, 0,
> +        MR_nondetstack_zone_size, MR_default_handler);
> +
> +    list = GC_MALLOC_UNCOLLECTABLE(sizeof(MR_MemoryZones));

Ditto.

> +MR_define_extern_entry(MR_pop_detstack_segment);
> +MR_define_extern_entry(MR_pop_nondetstack_segment);
> +
> +MR_BEGIN_MODULE(stack_segment_module)
> +    MR_init_entry_an(MR_pop_detstack_segment);
> +    MR_init_entry_an(MR_pop_nondetstack_segment);
> +MR_BEGIN_CODE
> +
> +MR_define_entry(MR_pop_detstack_segment);
> +#ifdef MR_STACK_SEGMENTS
> +{
> +    MR_MemoryZones  *list;
> +    MR_Word         *orig_sp;
> +    MR_Code         *orig_succip;
> +
> +    orig_sp = (MR_Word *) MR_stackvar(1);
> +    orig_succip = (MR_Code *) MR_stackvar(2);
> +
> +#ifdef  MR_DEBUG_STACK_SEGMENTS
> +    printf("restore old det segment: old zone %p, old sp %p\n",
> +        MR_CONTEXT(MR_ctxt_detstack_zone), MR_sp);
> +    printf("old sp: ");
> +    MR_printdetstack(MR_sp);
> +    printf("old succip: ");
> +    MR_printlabel(stdout, MR_succip);
> +#endif
> +
> +    MR_unget_zone(MR_CONTEXT(MR_ctxt_detstack_zone));
> +
> +    list = MR_CONTEXT(MR_ctxt_prev_detstack_zones);
> +    MR_CONTEXT(MR_ctxt_detstack_zone) = list->MR_zones_head;
> +    MR_CONTEXT(MR_ctxt_prev_detstack_zones) = list->MR_zones_tail;
> +    MR_CONTEXT(MR_ctxt_sp) = orig_sp;
> +    GC_FREE(list);

MR_GC_FREE

> +MR_define_entry(MR_pop_nondetstack_segment);
> +#ifdef MR_STACK_SEGMENTS
> +{
> +    MR_MemoryZones  *list;
> +    MR_Word         *orig_maxfr;
> +    MR_Code         *orig_succip;
> +
> +    orig_maxfr = (MR_Word *) MR_stackvar(1);
> +    orig_succip = (MR_Code *) MR_stackvar(2);
> +
> +#ifdef  MR_DEBUG_STACK_SEGMENTS
> +    printf("restore old nondet segment: old zone %p, old maxfr %p\n",
> +        MR_CONTEXT(MR_ctxt_nondetstack_zone), MR_maxfr);
> +    printf("old maxfr: ");
> +    MR_printnondetstack(MR_maxfr);
> +    printf("old succip: ");
> +    MR_printlabel(stdout, MR_succip);
> +#endif
> +
> +    MR_unget_zone(MR_CONTEXT(MR_ctxt_nondetstack_zone));
> +
> +    list = MR_CONTEXT(MR_ctxt_prev_nondetstack_zones);
> +    MR_CONTEXT(MR_ctxt_nondetstack_zone) = list->MR_zones_head;
> +    MR_CONTEXT(MR_ctxt_prev_nondetstack_zones) = list->MR_zones_tail;
> +    MR_CONTEXT(MR_ctxt_maxfr) = orig_maxfr;
> +    GC_FREE(list);

Ditto.

> Index: runtime/mercury_wrapper.c
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_wrapper.c,v
> retrieving revision 1.170
> diff -u -b -r1.170 mercury_wrapper.c
> --- runtime/mercury_wrapper.c	9 Oct 2006 06:40:25 -0000	1.170
> +++ runtime/mercury_wrapper.c	30 Oct 2006 05:49:57 -0000
> @@ -165,6 +170,18 @@
>  
>  double MR_heap_expansion_factor = 2.0;
>  
> +/*
> +** When we use stack segments, we reserve the last MR_stack_margin_size bytes
> +** of each stack segment for leaf procedures. This way, leaf procedures that
> +** do not need this much stack space can allocate their stack space *without*
> +** incurring the cost of a test.
> +**
> +** MR_stack_margin_size is never consulted directly; instead, its value is used
> +** to set the MR_zone_extend_threshold field in a stack's memory zone.
> +*/
> +
> +size_t      MR_stack_margin_size = 128;
> +

It might be worth checking that the user hasn't specified a detstack or
nondetstack size smaller than the margin size.

> Index: scripts/final_grade_options.sh-subr
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/scripts/final_grade_options.sh-subr,v
> retrieving revision 1.14
> diff -u -b -r1.14 final_grade_options.sh-subr
> --- scripts/final_grade_options.sh-subr	18 May 2005 05:45:00 -0000	1.14
> +++ scripts/final_grade_options.sh-subr	27 Oct 2006 16:07:28 -0000
> @@ -41,6 +41,15 @@
>  esac
>  
>  #
> +# .exts grade is not compatible with .stseg
> +#	(they are alternative ways of doing the same thing)
> +#
> +case $extend_stacks,$stack_segments in true,true)
> +	echo "--extend-stacks and --stack-segments are not compatible" 1>&2
> +	exit 1 ;;

--extend-stacks-when-needed

The rest looked fine, but some files had an inconsistent mix of tabs and
spaces so indentation was off when viewed with the default tab size.

Peter

--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------