[m-dev.] for review: MLDS->C back-end: efficiency improvements

[Peter Ross]

On Wed, Jan 17, 2001 at 11:35:22PM +1100, Fergus Henderson wrote:
> Estimated hours taken: 5
> 
> Some efficiency improvements to the MLDS->C back-end.
> In particular, use GCC's __builtin_setjmp/__builtin_longjmp(),
> and generate better code for switches whose default case is
> unreachable.
> 
> These changes also help to ensure that the MLDS->C back-end should
> generate exactly the same assembler code as the MLDS->GCC back-end,
> which makes it easier to debug the latter (by using `diff' on the
> generated `.s' files).
> 
> compiler/mlds_to_c.m:
> 	For unreachable default cases, use `MR_assert(0)' rather than
> 	`assert(0)'.  This improves efficiency, since `MR_assert' is
> 	disabled unless you compile with -DMR_LOWLEVEL_DEBUG.
> 	These checks were useful in debugging the MLDS back-end, but
> 	it's probably not worth keeping them enabled by default now.
> 
> 	Put the default case first, so that if it is empty (as it will
> 	be if the default is unreachable and MR_assert() is not enabled),
> 	it gets merged with.
> 

with what?

> 	Replace an obsolete comment about a problem with setjmp() and
> 	volatile with a comment explaining how that problem was fixed.
> 
> 	Generate calls to MR_builtin_setjmp() and MR_builtin_longjmp()
> 	rather than to setjmp() and longjmp().
> 
I would prefer MR_setjmp() and MR_longjmp() because that is the more
general case.

> runtime/mercury.h:
> 	Define MR_builtin_setjmp() and MR_builtin_longjmp().
> 	These expand to __builtin_setjmp()/__builtin_longjmp() for GNU C
> 	and to the standard setjmp()/longjmp() otherwise.
> 
> compiler/ml_code_gen.m:
> 	Add some comments about __builtin_setjmp() and __builtin_longjmp().
> 
> Workspace: /home/hg/fjh/gcc-cvs/gcc/mercury
> Index: compiler/ml_code_gen.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/ml_code_gen.m,v
> retrieving revision 1.72
> diff -u -d -r1.72 ml_code_gen.m
> --- compiler/ml_code_gen.m	2000/12/14 08:02:18	1.72
> +++ compiler/ml_code_gen.m	2001/01/17 12:25:52
> @@ -150,6 +150,7 @@
>  % There's several different ways of handling commits:
>  %	- using catch/throw
>  %	- using setjmp/longjmp
> +%	- using GCC's __builtin_setjmp/__builtin_longjmp
>  %	- exiting nested functions via gotos to
>  %	  their containing functions
>  %
> @@ -158,6 +159,17 @@
>  % The comments below show the MLDS try_commit/do_commit version first,
>  % but for clarity I've also included sample code using each of the three
>  % different techniques.
> +%
> +% Note that if we're using GCC's __builtin_longjmp(),
> +% then it is important that the call to __builtin_longjmp() be
> +% put in its own function, to ensure that it is not in the same
> +% function as the __builtin_setjmp().
> +% The code generation schema below does that automatically.
> +% We will need to be careful with MLDS optimizations to
> +% ensure that we preserve that invariant, though.
> +% (Alternatively, we could just call a function that
> +% calls __builtin_longjmp() rather than calling it directly.
> +% But that would be a little less efficient.)
>  %

I can see this being a gotcha that is going to get us at some stage.  I
would prefer to see some sort of check added to avoid this problem.
Maybe we should add a pass which traverses the MLDS just before output
and checks for any problems.

>  % If those methods turn out to be too inefficient,
>  % another alternative would be to change the generated

> Index: runtime/mercury.h
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury.h,v
> retrieving revision 1.32
> diff -u -d -r1.32 mercury.h
> --- runtime/mercury.h	2001/01/14 03:10:53	1.32
> +++ runtime/mercury.h	2001/01/17 12:13:13
> @@ -70,6 +70,22 @@
>  typedef void MR_CALL (*MR_Cont) (void *); /* for --no-gcc-nested-functions */
>  
>  /*
> +** The jmp_buf type used by MR_builtin_setjmp()
> +** to save the stack context when implementing commits.
> +*/
> +#ifdef __GNUC__
> +  /*
> +  ** For GCC, we use `__builtin_setjmp' and `__builtin_longjmp'.
> +  ** These are documented (in gcc/builtins.c in the GCC source code)
> +  ** as taking as their parameter a pointer to an array of five words. 

s/as taking as/as taking for/

reads slightly better to me.

> +  */
> +  typedef void *MR_builtin_jmp_buf[5];
> +#else
> +  /* Otherwise we use the standard jmp_buf type */
> +  typedef jmp_buf MR_builtin_jmp_buf;
> +#endif
> +
> +/*
>  ** The types uses to represent the Mercury builtin types,
>  ** MR_Char, MR_Float, MR_Integer, MR_String, and MR_ConstString,
>  ** are defined in mercury_types.h and mercury_float.h.
> @@ -277,6 +293,24 @@
>  /*
>  ** Macro / inline function definitions
>  */
> +
> +/*
> +** These macros expand to the either the standard setjmp()/longjmp()
> +** or to the GNU __builtin_setjmp() and __builtin_longjmp().
> +** The GNU versions are the same as the standard versions,
> +** except that they are more efficient, and that they have two
> +** restrictions:
> +**	1.  The second argument to __builtin_longjmp() must always be `1'.
> +**	2.  The call to __builtin_longjmp() must not be in the same
> +**	    function as the call to __builtin_setjmp().
> +*/
> +#ifdef __GNUC__
> +  #define MR_builtin_setjmp(buf)	__builtin_setjmp((buf))
> +  #define MR_builtin_longjmp(buf, val)	__builtin_longjmp((buf), (val))
> +#else
> +  #define MR_builtin_setjmp(buf)	setjmp((buf))
> +  #define MR_builtin_longjmp(buf, val)	longjmp((buf), (val))
> +#endif
>  
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------