[m-rev.] for review: fix mantis bug 56

[Julien Fischer]

On Mon, 26 May 2008, Zoltan Somogyi wrote:

> Fix a bug that prevented the compiler from being able to bootstrap in debugging
> grades. This was Mantis bug #56. (The bug was not affected by the presence or
> absence of trailing, or the issue of debug vs decldebug.)
>
> We have previously found that the problem could be eliminated by compiling
> polymorphism.m with --no-common-struct. I isolated the problem to the code
> we used to generate at the start of the else case in the only if-then-else
> in fixup_pred_polymorphism when --common-struct is enabled:
>
> 	MR_r1 = MR_sv(20);
>                /* Placing PredInfo1 */
>        MR_r1 = MR_sv(28);
>                /* Placing TypeInfo_34 (depth 1) */
>        MR_sv(28) = MR_sv(29);
>                /* Placing TypeInfo_37 */
>
> The bug was of course the second assignment to r1. It was caused not by
> anything done by common.m, but was a bug (an over-eager optimization)
> in the code generator that just happened to be tickled *extremely* rarely.
>
> compiler/var_locn.m:
> 	The bug was caused by a sometimes-false assumption. The assumption
> 	was that when we need to free up an lval because we are about to
> 	place a value in it, we can move it into the register preferred by
> 	follow_vars for the variable being moved, provided that register isn't
> 	locked.
>
> 	This is true when we are placing the arguments of a call, because the
> 	preferred location for any value being moved is either a stack slot
> 	(if the value is not an argument), or a register we haven't set yet
> 	(if the value is a following argument). It cannot be a register we have
> 	already set, because in that case there would be no need to move the
> 	value out of a later register.
>
> 	However, the assumption is false when placing resume vars between
> 	the stack and original resume maps, as in fixup_pred_polymorphism.
> 	In that case, we do not impose any particular order on the sequence
> 	of place_var operations required. The bug was that when we needed
> 	to free up a location (stackvar 28), we moved the value in it into r1,
> 	even though r1 already contained a value we needed.
>
> 	The fix is to make sure that we never pick a register currently in use.
> 	Since for calls, the in-use check should not fail, this should not
> 	lead to significantly worse code being generated.

That looks fine.

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