[m-rev.] for post-commit review: compatibility with gcc 3.4

[Fergus Henderson]

On 09-Jul-2004, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> Index: runtime/mercury_regs.h
..
> +** MR_save_mhal_registers(), MR_restore_mhal_registers(),

What does "mhal" standard for?
My first guess was that this was something to do with the HAL language.
A comment here might be helpful.

> +** The Mercury abstract machine registers consist of four groups.
...
> +** - The special purpose registers that are always stored in global variables,
> +**   namely
> +**
> +**	MR_sol_hp
> +**	MR_min_hp_rec
> +**	MR_min_sol_hp_rec
> +**	MR_global_hp
> +**
> +**	MR_trail_ptr		if trailing is enabled
> +**	MR_ticket_counter	if trailing is enabled
> +**	MR_ticket_high_water	if trailing is enabled
> +**
> +**	MR_gen_next		if minimal model is enabled
> +**	MR_gen_stack		if minimal model is enabled

The first four of those should only be used if conservative GC is not enabled.
It's probably worth mentioning that in a comment here.

> +#if defined(MR_THREAD_SAFE) && MR_NUM_REAL_REGS > 0
> +
> +  #define MR_engine_base_word		MR_count_usage(MR_SP_SLOT, MR_mr0)

The use of MR_SP_SLOT there looks wrong.
Shouldn't that be MR_ENGINE_BASE_SLOT, or something like that?

> +#define MR_saved_succip_word(save_area)		(save_area[MR_SI_SLOT])

The second occurrence of "save_area" there should be in parentheses,
otherwise e.g. "MR_saved_succip_word(*save_area_ptr)" would do the wrong
thing.  Likewise for all the other MR_saved_* macros.

...
> +/*
> +** The MR_save_registers() macro copies the physical machine registers
> +** and the global variables holding special purpose abstract machine registers
> +** to their corresponding slots in the MR_fake_reg array.
> +**
> +** MR_restore_registers() does the same transfer in the opposite direction.
> +**
> +** For speed, neither copies the special purpose registers that are known
> +** not to be needed in the current grade.
> +*/
> +
> +#define MR_save_registers() 						\
> +	do {								\
> +		MR_save_mhal_registers();				\
> +		MR_saved_sol_hp_word(MR_fake_reg) = (MR_Word)		\
> +			MR_sol_hp;					\
> +		MR_saved_min_hp_rec_word(MR_fake_reg) = (MR_Word)	\
> +			MR_min_hp_rec;					\
> +		MR_saved_min_sol_hp_rec_word(MR_fake_reg) = (MR_Word)	\
> +			MR_min_sol_hp_rec;				\
> +		MR_saved_global_hp_word(MR_fake_reg) = (MR_Word)	\
> +			MR_global_hp;					\
> +		MR_save_trail_registers();				\
> +		MR_save_mm_registers();					\
> +	} while (0)

The assignments of MR_*_hp_* here should be conditional on MR_CONSERVATIVE_GC
not being defined.

Also, I am a bit concerned about the impact of this change on the
efficiency of the C interface.  Copying the special registers that are
stored in global variables to/from MR_fake_reg may add quite a few loads
and stores to the cost of MR_save_registers()/MR_restore_registers().
I think that in many places where
MR_save_registers()/MR_restore_registers() is currently used, and
in particular for the C interface, it would be sufficient to just do
MR_save_mhal_registers()/MR_restore_mhal_registers().

Otherwise this change looks good!

-- 
Fergus Henderson                    |  "I have always known that the pursuit
                                    |  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
--------------------------------------------------------------------------