[m-rev.] LLDS accurate GC improvements

[Fergus Henderson]

On 22-Oct-2003, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> Various improvements for accurate GC in LLDS grades:

...and some more...

Estimated hours taken: 4
Branches: main

Various bug fixes for accurate GC in LLDS grades:

runtime/mercury_wrapper.c:
	Fix two bugs in my last change:
	- use MR_NATIVE_GC rather than the non-existent MR_ACCURATE_GC
	- use %lf rather than %f when calling scanf() for a double

runtime/mercury_accurate_gc.c:
	- Fix some bugs with traversing the nondet stack.
	- Make sure that we round the active heap size up to a multiple
	  of the page size, otherwise mprotect() won't work.
	- Avoid some casts by using type `MR_Code **' rather than
	  `MR_Word *' for the saved_success_location variable.
	- Comment out some unused code.

runtime/mercury_stacks.h:
	Add MR_curfr_slot_addr(), which is like MR_curfr_slot() except that
	it returns the slot's address, for use by mercury_accurate_gc.c.
	(Using "&MR_curfr_slot(...)" results in an "invalid lvalue" error
	from GCC, despite the use of MR_LVALUE_CAST in its definition.)

runtime/mercury_stack_trace.h:
	Add some comments.

runtime/mercury_stack_trace.c:
	Fix some incorrect indentation.

compiler/stack_layout.m:
	Fix a cut-and-paste error.

Workspace: /home/ceres/fjh/mercury
Index: compiler/stack_layout.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/stack_layout.m,v
retrieving revision 1.84
diff -u -d -r1.84 stack_layout.m
--- compiler/stack_layout.m	20 Oct 2003 07:29:11 -0000	1.84
+++ compiler/stack_layout.m	22 Oct 2003 07:50:53 -0000
@@ -1419,7 +1419,7 @@
 	stack_layout__locn_type_code(lval_hp, Val),
 	stack_layout__make_tagged_byte(3, Val, Byte).
 stack_layout__represent_lval_as_byte(sp, Byte) :-
-	stack_layout__locn_type_code(lval_succip, Val),
+	stack_layout__locn_type_code(lval_sp, Val),
 	stack_layout__make_tagged_byte(3, Val, Byte).
 
 :- pred stack_layout__make_tagged_byte(int::in, int::in, int::out) is semidet.
Index: runtime/mercury_accurate_gc.c
===================================================================
RCS file: /home/mercury1/repository/mercury/runtime/mercury_accurate_gc.c,v
retrieving revision 1.25
diff -u -d -r1.25 mercury_accurate_gc.c
--- runtime/mercury_accurate_gc.c	22 Oct 2003 05:56:07 -0000	1.25
+++ runtime/mercury_accurate_gc.c	22 Oct 2003 08:00:02 -0000
@@ -66,8 +66,8 @@
 */
 
 #ifndef MR_HIGHLEVEL_CODE
-static MR_Code	*saved_success = (MR_Code *) NULL;
-static MR_Word	*saved_success_location = (MR_Word *) NULL;
+static MR_Code	*saved_success = NULL;
+static MR_Code **saved_success_location = NULL;
 static MR_bool	gc_scheduled = MR_FALSE;
 static MR_bool	gc_running = MR_FALSE;
 #endif
@@ -345,7 +345,7 @@
 		fprintf(stderr, "GC scheduled again. Replacing old scheduling,"
 			" and trying to schedule again.\n");
 #endif
-		*saved_success_location = (MR_Word) saved_success;
+		*saved_success_location = saved_success;
 	}
 	gc_scheduled = MR_TRUE;
 
@@ -361,32 +361,35 @@
 		/*
 		** Save the old succip and its location.
 		*/
-		saved_success_location = &MR_based_stackvar(sp_at_signal,
-			number);
-		saved_success = (MR_Code *) *saved_success_location;
+		saved_success_location = (MR_Code **)
+			&MR_based_stackvar(sp_at_signal, number);
+		saved_success = *saved_success_location;
 	} else {
 		/*
 		** XXX we ought to also overwrite the redoip,
 		**     otherwise we might miss failure-driven loops
 		**     which don't contain any returns.
 		*/
-		saved_success_location = &MR_based_framevar(curfr_at_signal,
-			number);
-		saved_success = (MR_Code *) *saved_success_location;
+		/*
+		** Save the old succip and its location.
+		*/
+		assert(location == -1); /* succip is saved in succip_slot */
+		saved_success_location = MR_succip_slot_addr(curfr_at_signal);
+		saved_success = *saved_success_location;
 	}
 
 #ifdef MR_DEBUG_AGC_SCHEDULING
 	fprintf(stderr, "old succip: %ld (%lx) new: %ld (%lx)\n", 
 		(long) saved_success, (long) saved_success,
-		(long) ENTRY(mercury__garbage_collect_0_0),
-		(long) ENTRY(mercury__garbage_collect_0_0));
+		(long) MR_ENTRY(mercury__garbage_collect_0_0),
+		(long) MR_ENTRY(mercury__garbage_collect_0_0));
 #endif
 
 	/*
 	** Replace the old succip with the address of the
 	** garbage collector.
 	*/
-	*saved_success_location = (MR_Word) mercury__garbage_collect_0_0;
+	*saved_success_location = MR_ENTRY(mercury__garbage_collect_0_0);
 
 #ifdef MR_DEBUG_AGC_SCHEDULING
 	fprintf(stderr, "Accurate GC scheduled.\n");
@@ -542,8 +545,12 @@
 
         /* Get the type parameters from the stack frame. */
 
-	/* XXX We must pass NULL since the registers have not been saved */
-	/* XXX This is probably a bug; Tyson should look into it */
+	/*
+	** We must pass NULL here since the registers have not been saved;
+	** This should be OK, because none of the values used by a procedure
+	** will be stored in registers across a call, since we have no
+	** caller-save registers (they are all callee-save).
+	*/
         type_params = MR_materialize_type_params_base(label_layout,
             NULL, stack_pointer, current_frame);
         
@@ -587,7 +594,7 @@
 
 	{
 		MR_Long_Lval            location;
-		MR_Long_Lval_Type            type;
+		MR_Long_Lval_Type       type;
 		int                     number;
 
 		location = proc_layout->MR_sle_succip_locn;
@@ -595,11 +602,18 @@
 		number = MR_LONG_LVAL_NUMBER(location);
 		if (type != MR_LONG_LVAL_TYPE_STACKVAR) {
 			MR_fatal_error("can only handle stackvars");
-			}
-		
-		success_ip = (MR_Code *) MR_based_stackvar(stack_pointer, number);
-		stack_pointer = stack_pointer - 
-			proc_layout->MR_sle_stack_slots;
+		}
+		if (MR_DETISM_DET_STACK(proc_layout->MR_sle_detism)) {
+			success_ip = (MR_Code *)
+				MR_based_stackvar(stack_pointer, number);
+			stack_pointer = stack_pointer - 
+				proc_layout->MR_sle_stack_slots;
+		} else {
+			/* succip is saved in succip_slot */
+			assert(location == -1);
+			success_ip = MR_succip_slot(current_frame);
+			current_frame = MR_succfr_slot(current_frame);
+		}
 		label = MR_lookup_internal_by_addr(success_ip);
 	}
 
@@ -632,6 +646,7 @@
     */ 
     
     while (max_frame > MR_nondet_stack_trace_bottom) {
+#if 0
 	MR_bool registers_valid;
 	int frame_size;
 
@@ -655,6 +670,7 @@
 		fflush(NULL);
 	    }
 	}
+#endif
 	label = MR_lookup_internal_by_addr(MR_redoip_slot(max_frame));
 	stack_pointer = NULL; /* XXX ??? */
 
@@ -666,10 +682,14 @@
 		long_var_count = MR_long_desc_var_count(label_layout);
 		/* var_count = label_layout->MR_sll_var_count; */
 
-		/* 
-		** XXX We must pass NULL since the registers have not
-		** been saved. This is probably a bug; Tyson should look
-		** into it
+		/*
+		** We must pass NULL here since the registers have not been
+		** saved; This should be OK, because none of the values used
+		** by a procedure will be stored in registers across a call,
+		** since we have no caller-save registers (they are all
+		** callee-save).
+		** XXX Is it right to pass MR_redofr_slot(max_frame) here?
+		**     Why not just max_frame?
 		*/
 		type_params = MR_materialize_type_params_base(label_layout,
 		    NULL, stack_pointer, MR_redofr_slot(max_frame));
@@ -764,7 +784,9 @@
       ** (which defaults to two) times the current usage,
       ** or the size at which we GC'd last time, whichever is larger.
       */
-      gc_heap_size = (size_t) (MR_heap_expansion_factor * new_heap_usage);
+      gc_heap_size = MR_round_up(
+	      	(size_t) (MR_heap_expansion_factor * new_heap_usage),
+		MR_unit);
       if (gc_heap_size < old_heap_space) {
 	      gc_heap_size = old_heap_space;
       }
Index: runtime/mercury_stack_trace.c
===================================================================
RCS file: /home/mercury1/repository/mercury/runtime/mercury_stack_trace.c,v
retrieving revision 1.55
diff -u -d -r1.55 mercury_stack_trace.c
--- runtime/mercury_stack_trace.c	16 Apr 2003 12:37:39 -0000	1.55
+++ runtime/mercury_stack_trace.c	22 Oct 2003 06:56:21 -0000
@@ -233,8 +233,8 @@
         *stack_trace_sp_ptr = *stack_trace_sp_ptr -
             entry_layout->MR_sle_stack_slots;
     } else {
-            success = MR_succip_slot(*stack_trace_curfr_ptr);
-            *stack_trace_curfr_ptr = MR_succfr_slot(*stack_trace_curfr_ptr);
+        success = MR_succip_slot(*stack_trace_curfr_ptr);
+        *stack_trace_curfr_ptr = MR_succfr_slot(*stack_trace_curfr_ptr);
     }
 
     return MR_stack_walk_succip_layout(success, return_label_layout,
Index: runtime/mercury_stack_trace.h
===================================================================
RCS file: /home/mercury1/repository/mercury/runtime/mercury_stack_trace.h,v
retrieving revision 1.28
diff -u -d -r1.28 mercury_stack_trace.h
--- runtime/mercury_stack_trace.h	16 Apr 2003 12:37:39 -0000	1.28
+++ runtime/mercury_stack_trace.h	9 Oct 2003 10:51:23 -0000
@@ -118,8 +118,8 @@
 ** MR_stack_walk_step:
 **	This function takes the entry_layout for the current stack
 **	frame (which is the topmost stack frame from the two stack
-**	pointers given), and moves down one stack frame, setting the
-**	stack pointers to their new levels.
+**	pointers given), and moves down one stack frame, i.e. to the
+**	caller's frame, setting the stack pointers to their new levels.
 **
 **	return_label_layout will be set to the stack_layout of the
 **	continuation label, or NULL if the bottom of the stack has
@@ -135,6 +135,11 @@
 **
 **	If a MR_stack_walk_step encounters a problem, it will set problem_ptr
 **	to point to a string representation of the error.
+**
+**	Note that for nondetermistic code, this function will only
+**	traverse the success continuations (via MR_succfr),
+**	not the frames which represent failure continuations
+**	(which would be accessible via MR_redofr).
 */
 
 typedef enum {
Index: runtime/mercury_stacks.h
===================================================================
RCS file: /home/mercury1/repository/mercury/runtime/mercury_stacks.h,v
retrieving revision 1.39
diff -u -d -r1.39 mercury_stacks.h
--- runtime/mercury_stacks.h	2 May 2003 21:44:16 -0000	1.39
+++ runtime/mercury_stacks.h	22 Oct 2003 07:27:38 -0000
@@ -160,6 +160,7 @@
 					((MR_Word *) (fr))[MR_REDOIP])
 #define	MR_redofr_slot(fr)	MR_LVALUE_CAST(MR_Word *,		\
 					((MR_Word *) (fr))[MR_REDOFR])
+#define	MR_succip_slot_addr(fr)	(&(((MR_Code **) (fr))[MR_SUCCIP]))
 #define	MR_succip_slot(fr)	MR_LVALUE_CAST(MR_Code *,		\
 					((MR_Word *) (fr))[MR_SUCCIP])
 #define	MR_succfr_slot(fr)	MR_LVALUE_CAST(MR_Word *,		\
Index: runtime/mercury_wrapper.c
===================================================================
RCS file: /home/mercury1/repository/mercury/runtime/mercury_wrapper.c,v
retrieving revision 1.126
diff -u -d -r1.126 mercury_wrapper.c
--- runtime/mercury_wrapper.c	22 Oct 2003 05:56:08 -0000	1.126
+++ runtime/mercury_wrapper.c	22 Oct 2003 06:24:42 -0000
@@ -103,7 +103,7 @@
 ** instead GCs are scheduled, based on MR_heap_margin_size (see below),
 ** by explicit calls to MR_GC_check()
 */
-#if defined(MR_ACCURATE_GC) && !defined(MR_HIGHLEVEL_CODE)
+#if defined(MR_NATIVE_GC) && !defined(MR_HIGHLEVEL_CODE)
   #ifdef MR_DEBUG_AGC_SMALL_HEAP
     size_t		MR_heap_zone_size =	  32;
   #else
@@ -1030,7 +1030,7 @@
 			break;
 
 		case MR_HEAP_EXPANSION_FACTOR:
-			if (sscanf(MR_optarg, "%f", &MR_heap_expansion_factor)
+			if (sscanf(MR_optarg, "%lf", &MR_heap_expansion_factor)
 					!= 1)
 			{
 				usage();

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------