[m-rev.] for review: --no-inline-builtins

[Fergus Henderson]

On 29-Jan-2003, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> 
> Add an option `--no-inline-builins', which causes builtins to
> be generated as calls to out-of-line procedures. This can be
> useful for debugging, as without this option the execution of
> builtins is not traced.
> 
> On earth, a compiler built in grade asm_fast.gc.tr.debug
> takes 36.8s to run `mmc -C -I ../analysis/ hlds.make_hlds'.
> When the compiler is built with `--no-inline-builtins',
> this is increased to 38.6s.
> 
> The size of the compiler built in grade asm_fast.gc.tr.debug
> increases from 45.0MB to 46.6MB.

That looks good.  I think this option should be enabled by
default if execution tracing is requested (e.g. for debug
grades), unless `--trace-optimized' is specified.

Oh, looking at the diff, I think you did that already.
But the log message, NEWS file entry, and option documentation
didn't make that clear.

> --- tests/debugger/Mmakefile	15 Nov 2002 04:50:43 -0000	1.92
> +++ tests/debugger/Mmakefile	29 Jan 2003 02:40:01 -0000
> @@ -101,8 +101,20 @@
>      SENSITIVE_PROGS :=
>  endif
>  
> +# The no_inline_builtins test only works if the library is
> +# built with execution tracing enabled. Adding a `.exp2' file
> +# to allow it to be run in other grades would mean that we
> +# wouldn't detect a regression which caused builtins not to
> +# be traced with `--no-inline-builtins'.
> +ifeq "$(findstring debug,$(GRADE))" "debug"
> +	DEBUG_GRADE_PROGS = no_inline_builtins
> +else 
> +	DEBUG_GRADE_PROGS =
> +endif

I think it's also worth covering the case when
`--no-inline-builtins' is specified and debugging isn't.
This could be done by copying the no_inline_builtins.m
test cases to another file, no_inline_builtins2.m,
and changing the `else' case there to use no_inline_builtins2.m.
The only difference between the two files would be that they
would be accompanied by different `.exp' files.

> Index: tests/debugger/queens.exp2
> ===================================================================
> RCS file: /home/mercury1/repository/tests/debugger/queens.exp2,v
> retrieving revision 1.5
> diff -u -u -r1.5 queens.exp2
> --- tests/debugger/queens.exp2	17 Jan 2003 05:56:56 -0000	1.5
> +++ tests/debugger/queens.exp2	29 Jan 2003 02:55:34 -0000
> @@ -1,45 +1,48 @@
> -       1:      1  1 CALL pred queens.main/2-0 (cc_multi) queens.m:17
> +      E1:     C1  1 CALL pred queens.main/2-0 (cc_multi) queens.m:17
>  mdb> echo on
>  Command echo enabled.
> +mdb> register --quiet
>  mdb> retry 1
>  not that many ancestors
>  mdb> print *
>         DCG_0 (arg 1)          	state('<<c_pointer>>')
> -mdb> 
> -       2:      2  2 CALL pred queens.data/1-0 (det) queens.m:39 (queens.m:15)
> +mdb> b data
> + 0: + stop  interface pred queens.data/1-0 (det)
> +mdb> continue
> +      E2:     C2  2 CALL pred queens.data/1-0 (det) queens.m:39 (queens.m:15)

Huh?  Where did all these differences come from?

> +++ tests/debugger/declarative/args.exp	29 Jan 2003 03:57:11 -0000
> @@ -11,7 +11,15 @@
>  mdb> dd
>  p(1, 16, 3, 20, 5)
>  Valid? no
> -my_succeed
> ++(1, 3) = 4
> +Valid? yes
> +*(3, 5) = 15
> +Valid? yes
> ++(1, 15) = 16
> +Valid? yes
> +*(4, 5) = 20
> +Valid? yes
> +semidet_succeed

You know, I think the declarative debugger really ought to by default
assume that everything in the standard library is correct.
Having the declarative debugger ask you if 1 + 3 = 4 is not a
usability improvement!  (Shades of 1984...)

However, that should be a separate patch.

> --- tests/debugger/declarative/gcf.m	2 Jun 1999 07:28:55 -0000	1.1
> +++ tests/debugger/declarative/gcf.m	29 Jan 2003 07:13:45 -0000
> @@ -3,7 +3,7 @@
>  :- import_module io.
>  :- pred main(io__state::di, io__state::uo) is cc_multi.
>  :- implementation.
> -:- import_module std_util, int.
> +:- import_module int, std_util.

That looks unnecessary.

----------

Apart from the issues mentioned above, this change looks good.

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