[m-dev.] for review: Aditi [3]

[Simon Taylor]

Hi,
 
> > --- modules.m	1998/07/01 04:09:39	1.81
> > +++ modules.m	1998/07/02 01:45:46
> > @@ -503,6 +503,8 @@
> >                  ; Ext = ".hlds_dump"
> >                  ; Ext = ".dependency_graph"
> >                  ; Ext = ".order"
> > +                ; Ext = ".rla"
> > +                ; Ext = ".rl_dump"
> >                  % Mmake targets
> >                  ; Ext = ".clean"
> >                  ; Ext = ".clean_nu"
> 
> You should update the documentation in doc/user_guide.texi
> about file naming conventions.

I've documented `.rlo' (we don't document `.hlds_dump....' in that
section so `.rla' and `.rl_dump' should't appear either).

> 
> > @@ -787,8 +789,8 @@
> >          ).
> >  
> >  % pragma `obsolete', `terminates', `does_not_terminate' 
> > -% `termination_info' and `check_termination' declarations
> > -% are supposed to go in the interface,
> > +% `termination_info', `check_termination' and Aditi pragma
> > +% declarations are supposed to go in the interface,
> ...
> > +pragma_allowed_in_interface(memo(_, _), no).
> > +pragma_allowed_in_interface(no_memo(_, _), no).
> ...
> > +pragma_allowed_in_interface(supp_magic(_, _), no).
> > +pragma_allowed_in_interface(context(_, _), no).
> > +pragma_allowed_in_interface(naive(_, _), no).
> > +pragma_allowed_in_interface(psn(_, _), no).
> 
> The code here isn't consistent with the comment.

That's a bug in the comment - the optimization pragmas should
go in the implementation.

> 
> > @@ -2106,6 +2121,12 @@
> >          io__write_string(DepStream, "\n"),
> >  
> >          io__write_string(DepStream, MakeVarName),
> > +        io__write_string(DepStream, ".rlos = "),
> > +        write_compact_dependencies_list(Modules, "$(rlos_subdir)", ".rlo",
> > +                                        Basis, DepStream),
> > +        io__write_string(DepStream, "\n"),
> > +
> > +        io__write_string(DepStream, MakeVarName),
> >          io__write_string(DepStream, ".pic_os = "),
> >          write_compact_dependencies_list(Modules, "$(os_subdir)",
> 
> Hmm, there is some argument for only emitting that stuff in the dependency
> files if `--aditi' or something like that is specified.
> I'm not sure what the best approach is here.

I'd rather not have to `mmake depend' just because I changed an option.
(Actually you need to mmake clean in this case because the already generated
C files won't contain the (empty) RL code constants).

> 
> options.m:
> > +        inline_alloc            -       bool(no),
> > +% RL    - not yet implemented
> > +        optimize_rl             -       bool(no),
> > +        optimize_rl_cse         -       bool(no),
> > +        optimize_rl_invariants  -       bool(no),
> > +        detect_rl_streams       -       bool(no)
> 
> Is the comment still correct?

I won't be committing those passes just yet.

> Actually the help messages should (1) be commented out for now
> and (2) match the description in the Mercury user's guide.

OK.
 
> >  polymorphism__process_pred(PredId, ModuleInfo0, ModuleInfo, IO0, IO) :-
> >          module_info_pred_info(ModuleInfo0, PredId, PredInfo),
> > -        pred_info_module(PredInfo, PredModule),
> > -        pred_info_name(PredInfo, PredName),
> > -        pred_info_arity(PredInfo, PredArity),
> >          (
> > -                polymorphism__no_type_info_builtin(PredModule,
> > -                        PredName, PredArity) 
> > +                (
> > +                        % Leave Aditi aggregates alone, since
> > +                        % calls to them must be monomorphic.
> > +                        % Other Aditi procedures should still be processed
> > +                        % to remove complicated unifications and
> > +                        % lambda expressions.
> > +                        hlds_pred__pred_info_is_aditi_aggregate(PredInfo)
> 
> If calls to them must be monomorphic, where is this checked?

The input query must be an Aditi relation. Currently we don't check that,
since the aggregate syntax is a bit up in the air anyway. For example, it
might be nice to have Aditi aggregates as a separate goal type to allow any
number of group-by and non-group-by variables (currently we only allow one
of each), and to make sure the input closures are constants, not passed
separately, which is required for code generation. I've added an XXX to
magic_util.m where the check should be done.

> If the type declaration for all Aditi aggregates is monomorphic,
> then there should be no harm in processing them -- correct?

The type declaration is polymorphic 
(from /home/mercury1/stayl/mercury/extras/aditi/aditi.m):
:- pred aditi__aggregate_compute_initial(pred(GrpBy, NonGrpBy), 
                pred(GrpBy, NonGrpBy, Acc, Acc),
                pred(GrpBy, NonGrpBy, Acc), GrpBy, Acc).

All calls must be monomorphic.
I'd rather not introduce type_infos into Aditi procedures,
since they just need to be stripped out again later.

Simon.