for review: fix to tracing for machines with transient regs

Fergus Henderson fjh at cs.mu.OZ.AU
Mon Feb 9 20:16:57 AEDT 1998


On 08-Feb-1998, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:

> Subject: for review: fix to tracing for machines with transient regs

Something like that would be good as a summary at the top of the log message.
E.g.

	Fix a bug that caused tracing to fail on machines with
	register windows (e.g. SPARCs).

> Index: compiler/trace.m
> ===================================================================
> +	SaveStmt = "save_transient_regs_to_mem(MR_trace_transient_save);\n",
>  	string__append_list([
> -		"MR_trace(",
> -		"(const Word *) &mercury_data__stack_layout__", LabelStr, Comma,
> +		"MR_trace((const MR_stack_layout_entry *) ",
> +		"&mercury_data__stack_layout__", LabelStr, Comma,

I suggest you put one or more newlines in there, to avoid
generating code with very long lines.

> +++ mercury_trace.c	1998/02/07 08:58:24
>  /*
> +** Since the call to MR_trace will clobber the transient registers
> +** on architectures that have them, the caller will save them first.
> +** The registers will be restored several times, with calls to the
> +** browser intervening between saves. Saving the transient registers
> +** in the fake_reg array would be incorrect, since the browser will
> +** clobber fake_reg. We therefore provide our own area.
> +**
> +** Warning: if any part of the the browser function is itself traced,
> +** MR_trace_transient_save will be overwritten, leading to a crash later on.

Ouch.

That would mean that we can't use the debugger to debug the Mercury library.

How about having MR_trace() copy MR_trace_transient_save to a local
array allocated on the C stack?  It can pass down a pointer to this
array to any routines that need to restore from it.  The array is only
MAX_REAL_REG Words long, so I think this double copying would not cause
any significant efficiency problem.

Actually this would also avoid the need for having an MR_trace_transient_save
global variable at all: code which calls MR_trace could just the transient
registers in the fake_reg array, and MR_trace would copy them from there to
its local transient_regs array.


Apart from that problem, the code you have is fine, and better
than the code currently in the repository, so if you want to commit
this change now and have you or I do the above-described improvement later,
that would be OK with me.

Cheers,
	Fergus.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>   |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>   |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3         |     -- the last words of T. S. Garp.



More information about the developers mailing list