[m-dev.] For Review: Bytecode interpreter

[Fergus Henderson]

On 13-Feb-2001, Levi Cameron <l.cameron2 at ugrad.unimelb.edu.au> wrote:
> Now that all in tests/general (except floats) run happily....
> 
> Estimated hours taken: 180
> 
>    compiler:
>  Added extra argument to test instruction (string comparisons were
>  being treated as integer comparisons; properly deals with different
>  atomic type unifications now)
>  Changed bytecode stub functions
>    interpreter:
>  Cleaned up comments
>  Forked part of mb_machine to mb_exec
>  Added test cases to repository
>  Added support for submodules
>  Added support for nondet procedures
>  Added support for cc_xxx procedures
>  Finished higher order calls
>  Added (very basic) debug interface
>  Added support for type information
>  Added memory corruption checking
>  Changed machine state dump formatting
>  Fixed bug in nested switches
>  Resolved builtin__unify and builtin_compare failures
>  Now passes all tests/general/* test cases except those with floats
>  Modified bytecode tags generation so .c & .m tag files are separate
>  Header usage rationalised

When the summary is this long, there should be a short summary at the
top, even if it is just something very general like "More work on the
bytecode interpreter".

The third-last line is not a change, and so it should be listed
separately.

So I suggest the following instead:

  More work on the bytecode interpreter.
  With these changes, it now passes all tests/general/* test cases
  except those with floats.

  Changes to the compiler:
  - Added extra argument to test instruction (string comparisons were
    being treated as integer comparisons; properly deals with different
    atomic type unifications now)
  - Changed bytecode stub functions

  Changes to the bytecode interpreter:
  - Cleaned up comments
  - Forked part of mb_machine to mb_exec
  - Added support for submodules
  - Added support for nondet procedures
  - Added support for cc_xxx procedures
  - Finished higher order calls
  - Added (very basic) debug interface
  - Added support for type information
  - Added memory corruption checking
  - Changed machine state dump formatting
  - Fixed bug in nested switches
  - Resolved builtin__unify and builtin_compare failures
  - Modified bytecode tags generation so .c & .m tag files are separate
  - Header usage rationalised

  Changes to test suite:
  - Added test cases for the bytecode interpreter.

> bytecode/Mmakefile
> 	Modified bytecode tags generation so .c & .m tag files are separate
> 	mb_machine split into mb_exec
> 	test file renamed to simple.m (copy over tests/simple??.m to test)

Please punctuate your log messages: there should be a colon after each
file name (this is important for automated parsing of log messages),
and a full stop at the end of each sentence.

> bytecode/test/simple??.m
> 	Various test files - just to check that it doesn't crash
> 	(Most do not output anything & must be verified by stepping through
> 	manually)

Those should probably go in tests/bytecode rather than bytecode/test.

> compiler/bytecode.m
> compiler/bytecode_gen.m
> 	Added extra argument to test instruction (otherwise
> 	string comparisons would be treated as integer comparisons

Missing ')'.

> Index: bytecode/Mmakefile
> +tags: $(ALL_CFILES) $(ALL_HDRS) tags2
> +	ctags $(TAGFLAGS) $(ALL_CFILES) $(ALL_HDRS) $(MERCURY_SYSTEM)
>  
> +tags2: $(ALL_MFILES)
> +	mtags $(TAGFLAGS) $(ALL_MFILES) $(MERCURY_SYSTEM)
> +	mv tags tags2

The options to ctags and mtags will probably be different,
so they should be different variables (CTAGSFLAGS and MTAGSFLAGS).

>  .PHONY: realclean_local
>  realclean_local:
> -	rm -f tags test
> +	rm -f tags tags2 *.mbc *.bytedebug
> +# XXX: The dependencies in mmake ignore .mbc and .bytedebug files
> +# so we have to manually delete them. We delete all bytecode files
> +# because it will leave submodule files if we don't

I don't understand the comment about leaving submodule files
(what are they?).

>  /* allocate space for a new string */
>  MB_CString	MB_str_new(MB_Word len);	/* len is w/o null terminator */

I suggest spelling out `w/o'; quite a few of the people reading the
Mercury source code may not be native English speakers.

> +++ simple99.m	Thu Feb  8 15:30:50 2001
> @@ -0,0 +1,47 @@
> +% simple99
> +
> +:- module simple.

The name here should probably match the file name.
Likewise for the other test cases.
I think this may be needed to get the test cases
to work with Mmake.

> Index: compiler/bytecode.m
...
> +++ compiler/bytecode_gen.m	2001/02/13 06:43:09
>  bytecode_gen__unify(simple_test(Var1, Var2), _, _, ByteInfo, Code) :-
>  	bytecode_gen__map_var(ByteInfo, Var1, ByteVar1),
>  	bytecode_gen__map_var(ByteInfo, Var2, ByteVar2),
> -	Code = node([test(ByteVar1, ByteVar2)]).
> +	bytecode_gen__get_var_type(ByteInfo, Var1, Var1Type),
> +	bytecode_gen__get_var_type(ByteInfo, Var2, Var2Type),
> +
> +	(	type_to_type_id(Var1Type, TypeId1, _),
> +		type_to_type_id(Var2Type, TypeId2, _)
> +	->	(	TypeId2 = TypeId1
> +		->	true
> +		;	error("unexpected simple_test between different types")
> +		),
> +		TypeId = TypeId1
> +	;	error("failed lookup of type id")
> +	),

s/true/TypeId = TypeId1/
and then delete the TypeId = TypeId1 below.

I haven't reviewed the changes to the bytecode directory yet.
But I'm happy with the changes to the compiler directory,
and I'm sure the changes to the bytecode directory are going to be
an improvement on the current status quo there, so I'm happy for you
to commit your changes once the above issues are addressed.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "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-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
--------------------------------------------------------------------------