[m-rev.] for review: stack slot optimization, part 6

[Fergus Henderson]

On 09-Mar-2002, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> Index: compiler/store_alloc.m
...
> @@ -114,154 +138,154 @@
>  	% should not be included in the store map.
>  	set__union(Liveness4, PostBirths, Liveness),
>  	(
> +		( Goal = switch(_Var, _CanFail, _Cases)
> +		; Goal = if_then_else(_Vars, _Cond, _Then, _Else)
> +		; Goal = disj(_Disjuncts)
> +		)

Use goal_util__goal_is_branched.

> Index: compiler/trace_params.m
...
>  	% These functions check for various properties of the trace level.
>  :- func trace_level_is_none(trace_level) = bool.
> +:- func trace_level_needs_input_vars(trace_level) = bool.

This change was not mentioned in the log message.

> +++ compiler/notes/compiler_design.html	8 Mar 2002 04:05:08 -0000
> @@ -653,12 +653,25 @@
>  			in the HLDS goal_info.
>  		<dt> allocation of stack slots
>  			<dd>
> -			This is done by live_vars.m, which works
> -			out which variables need to be saved on the
> -			stack when (trace.m determines what variables
> -			are needed for debugging purposes).
> -			It then uses graph_colour.m to determine
> -			a good allocation of variables to stack slots.
> +			This is done by stack_alloc.m, with the assistance of
> +			the following modules:
> +
> +			<ul>
> +			<li> live_vars.m works out which variables need
> +			to be saved on the stack when.
> +			<li> trace_params.m determines what fixed slots
> +			are needed for debugging purposes.
> +			<li> trace.m determines what variables
> +			are needed for debugging purposes.

That makes it sound like `trace_params.m' and `trace.m' are
subroutines of stack slot allocation.

The description here should distinguish between modules
whose whole function is as subroutine of stack slot allocation,
and modules which happen to have a small amount of functionality
that is used by stack slot allocation.

> Index: tools/compare_sp
> ===================================================================
> RCS file: tools/compare_sp
> diff -N tools/compare_sp
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ tools/compare_sp	19 Feb 2002 08:33:00 -0000
> @@ -0,0 +1,77 @@
> +#!/usr/bin/awk -f
> +#
> +# This file is the heart of the compare_stacks. It is not intended
> +# for use from the command line.

... nevertheless, it should be documented,
so that readers can understand how the code for
compare_stacks works.

> +$1 == "XYZZY" {
> +		base = $2 "";
> +		new = $3 "";
> +		next;
> +	}

This is especially cryptic.

Why "XYZZY"??
Why the ""'s here?
What do the variables "base" and "new" represent?

> Index: tools/compare_stacks
> ===================================================================
> RCS file: tools/compare_stacks
> diff -N tools/compare_stacks
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ tools/compare_stacks	19 Feb 2002 08:33:21 -0000
> @@ -0,0 +1,11 @@
> +#!/bin/sh
> +#
> +# XXX

This needs to be documented.

> +( echo XYZZY $2 $3; cat SP.$1.$2 SP.$1.$3 ) | compare_sp

Why don't you just use `NR == 1' rather than `$1 == "XYZZY"'
in compare_sp?

> +++ tools/extract_incr_sp	31 Dec 2001 05:32:06 -0000
> @@ -0,0 +1,55 @@
> +#!/usr/bin/awk -f

This tools should be documented.

> Index: tools/sum_incr_sp
> +++ tools/sum_incr_sp	31 Dec 2001 05:32:06 -0000
> @@ -0,0 +1,22 @@
> +#!/bin/sh

Likewise.

Otherwise this change looks fine.

-- 
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
--------------------------------------------------------------------------