[m-rev.] for review: term size profiling

[Zoltan Somogyi]

On 09-Oct-2003, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> On 01-Oct-2003, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> > tests/valid/Mmakefile:
> > 	Disable Aditi tests in term profiling grades, since Aditi will never
> > 	support term size profiling.
> 
> This shouldn't be necessary.

What shouldn't be necessary? Without those changes in the Makefile, those
tests failed.

> > Index: browser/program_representation.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/browser/program_representation.m,v
> > retrieving revision 1.9
> > diff -u -b -r1.9 program_representation.m
> > --- browser/program_representation.m	30 May 2003 07:41:11 -0000	1.9
> > +++ browser/program_representation.m	27 Aug 2003 12:11:44 -0000
> > @@ -233,9 +233,15 @@
> >  	).
> >  
> >  call_is_primitive(ModuleName, PredName) :-
> > +	(
> >  	ModuleName = "builtin",
> >  	( PredName = "unify"
> >  	; PredName = "compare"
> > +		)
> > +	;
> > +		ModuleName = "profiling_builtin"
> > +	;
> > +		ModuleName = "term_size_prof_builtin"
> >  	).
> 
> Fix the indentation here (or even better use separate clauses for each case).

The indentation is fine in the source file. Diff -b produces output like this
when a change indents existing code.

> > Index: compiler/mlds_to_il.m
> > ===================================================================
> > @@ -2508,10 +2497,20 @@
> >  		)
> >  	;
> >  		( already_boxed(SrcILType) ->
> > -			( SrcType = mercury_type(_, user_type, _) ->
> > +			(
> > +				SrcType = mercury_type(_, TypeCategory, _),
> > +				% XXX Consider whether this is the right way
> > +				% to handle type_infos, type_ctor_infos,
> > +				% typeclass_infos and base_typeclass_infos.
> > +				( TypeCategory = user_ctor_type
> > +				; is_introduced_type_info_type_category(
> > +					TypeCategory) = yes
> > +				)
> > +			->
> >  				% XXX we should look into a nicer way to
> >  				% generate MLDS so we don't need to do this
> > -				% XXX This looks wrong for --high-level-data. -fjh.
> > +				% XXX This looks wrong for --high-level-data.
> > +				% -fjh.
> >  				Instrs = tree__list([
> >  					comment_node(
> >  						"loading out of an MR_Word"),
> 
> Fergus should probably take a look at this, I don't know much about
> this code.

That's probably a good idea.

> >From the log message:
> > compiler/prog_util.m:
> > 	Add predicates and functions that return the sym_names of the modules
> > 	needed by term size profiling and some that could be needed later.
>  
> It's probably better to add them if/when they are needed.

OK.

> > Index: compiler/rl_key.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/compiler/rl_key.m,v
> > retrieving revision 1.14
> > diff -u -b -r1.14 rl_key.m
> > --- compiler/rl_key.m	27 May 2003 05:57:19 -0000	1.14
> > +++ compiler/rl_key.m	30 Sep 2003 08:08:03 -0000
> > @@ -131,9 +131,16 @@
> >  			list__member(ArgBound, ArgBounds),
> >  			ArgBound \= var - _
> >  		),
> > -		classify_type(Type, ModuleInfo, TypeClass),
> > -		( TypeClass = user_type
> > -		; TypeClass = enum_type
> > +		TypeCategory = classify_type(ModuleInfo, Type),
> > +		( TypeCategory = user_ctor_type
> > +		; TypeCategory = enum_type
> > +		  % XXX The next four categories used to be part of the
> > +		  % user_ctor_type category; we should think about whether
> > +		  % they should be included here.
> > +		; TypeCategory = type_ctor_info_type
> > +		; TypeCategory = type_info_type
> > +		; TypeCategory = typeclass_info_type
> > +		; TypeCategory = base_typeclass_info_type
> >  		),
> >  		module_info_types(ModuleInfo, Types),
> >  		type_to_ctor_and_args(Type, TypeCtor, _),
> 
> Type-infos and typeclass-infos can't occur here (Aditi predicates
> must be monomorphic).

OK, I deleted those four disjuncts.

> > +		generate_size_var(SizeVar0, KnownSize, Context,
> > +			SizeVar, SizeGoals, !Info),
> > +		% The increment_size primitive doesn't actually use the
> > +		% type_info passed to it, so give it the cheapest type_info
> > +		% we can build, which is a zero-arity type_ctor_info such as
> > +		% that of void.
> > +		make_type_info(Context, void_type, TypeInfoVar,
> > +			TypeInfoGoals, !Info),
> > +		TermSizeProfBuiltin = mercury_term_size_prof_builtin_module,
> > +		goal_util__generate_simple_call(TermSizeProfBuiltin,
> > +			"increment_size", predicate,
> > +			[TypeInfoVar, Var, SizeVar], only_mode, det,
> > +			yes(impure), [], !.Info ^ module_info,
> > +			Context, UpdateGoal),
> 
> You should cast TypeInfoVar to the correct type before passing it
> to increment_size.

I'll think about it.

> Alternatively, you could make increment_size a 
> no_type_info_builtin.

It isn't a builtin, and our mechanisms for handling builtins are currently
not flexible enough to make it a builtin.

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