[m-dev.] for review: merge HAL branch onto main branch

David Jeffery dgj at cs.mu.OZ.AU
Tue Mar 13 14:40:29 AEDT 2001


On 23-Feb-2001, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> On 23-Feb-2001, David Jeffery <dgj at cs.mu.OZ.AU> wrote:
> > @@ -2055,6 +2058,12 @@
> >                  "\tThis is necessary for interfacing with constraint solvers,",
> >                  "\tor for backtrackable destructive update.",
> >                  "\tThis option is not yet supported for the IL or Java back-ends.",
> > +                "--reserve-tag\t\t\t(grade modifier: `.rt')",
> > +                "\tReserve a tag in the data representation of the generated ",
> > +                "\tcode. This tag is intended to be used to give an explicit",
> > +                "\treprestation to free variables.",
> 
> s/represtation/representation/

Fixed.

> > +                error("type_ctor_info__make_enum_tables: no enums in .rt grade")
> 
> It would be better to use unexpected/2 from error_util.m here.

I've changed that to:
			unexpected("type_ctor_info", "enum in .rt grade")

> > +:- pred type_is_single_ctor_single_arg(list(constructor), sym_name, 
> > +        maybe(sym_name), type).
> > +:- mode type_is_single_ctor_single_arg(in, out, out, out) is semidet.
> > +
> > +type_is_single_ctor_single_arg(Ctors, Ctor, MaybeSymName, ArgType) :-
> > +        Ctors = [SingleCtor],
> > +        SingleCtor = ctor(ExistQVars, _Constraints, Ctor, 
> > +                [MaybeSymName - ArgType]),
> > +        ExistQVars = [].
> 
> A comment here explaining the `ExistQVars = []' test would help,
> e.g.
> 	
> 	% If there are any existentially quantified type variables,
> 	% then the type will contain hidden fields holding the type_infos
> 	% and/or typeclass infos for those type variables,
> 	% so it won't be a single-argument type.

Done.

> > Index: compiler/unify_proc.m
> > ===================================================================
> > RCS file: /home/staff/zs/imp/mercury/compiler/unify_proc.m,v
> > retrieving revision 1.91
> > diff -u -t -r1.91 unify_proc.m
> > --- compiler/unify_proc.m	2000/11/06 08:28:40	1.91
> > +++ compiler/unify_proc.m	2001/02/12 03:30:07
> > @@ -719,8 +719,22 @@
> >                          unify_proc__quantify_clauses_body([H1, H2], Goal,
> >                                  Context, Clauses)
> >                  ;
> > -                        unify_proc__generate_du_unify_clauses(Ctors,
> > -                                H1, H2, Context, Clauses)
> > +                        % Check to see if it's a single zero-arity functor.
> > +                        % This hack is required for --reserve-tag, which
> > +                        % disables enumeration types, so that the compiler
> > +                        % does not warn about inferring the unify to be
> > +                        % det rather than semidet.
> > +                        ( { Ctors = [ctor(_ExistQVars, _Constraints, _FunctorName, [])] , semidet_fail } ->
> > +                                % We must at least pretend a unification is
> > +                                % required?
> > +                                { create_atomic_unification(H1, var(H2),
> > +                                        Context, explicit, [], Goal) },
> > +                                unify_proc__quantify_clauses_body([H1, H2],
> > +                                        Goal, Context, Clauses)
> > +                        ;
> > +                                unify_proc__generate_du_unify_clauses(Ctors,
> > +                                        H1, H2, Context, Clauses)
> > +                        )
> 
> The call to semidet_fail means that that code is disabled.
> Did you intend to commit this bit as is?
> 
> The documentation should explain why the code is disabled.
> There should probably be an XXX.
> The condition there should be written over multiple lines.

Oops. I'm not commiting that change.

The code is buggy (it introduces an infinite loop in the generated code) and
the warning that it is trying to avoid doesn't seem to happen anyway!

> > +#ifdef MR_RESERVE_TAG
> 
> That configuration macro should be documented in
> runtime/mercury_conf_param.h.

I have made this change:

Index: mercury_conf_param.h
===================================================================
RCS file: /home/staff/zs/imp/mercury/runtime/mercury_conf_param.h,v
retrieving revision 1.46
diff -u -t -r1.46 mercury_conf_param.h
--- mercury_conf_param.h        2001/02/19 02:07:15     1.46
+++ mercury_conf_param.h        2001/03/13 01:10:51
@@ -50,6 +50,7 @@
 ** NO_TYPE_LAYOUT
 ** BOXED_FLOAT
 ** MR_USE_TRAIL
+** MR_RESERVE_TAG
 ** MR_USE_MINIMAL_MODEL
 **      See the documentation for
 **              --high-level-code
@@ -63,6 +64,7 @@
 **              --no-type-layout
 **              --unboxed-float
 **              --use-trail
+**              --reserve-tag
 **              --use-minimal-model
 **      (respectively) in the mmc help message or the Mercury User's Guide.
 **                                                                             

And added this to the log message:
-----
runtime/mercury_conf_param.h:
        Document the macro MR_RESERVE_TAG                                       
-----

> > +++ runtime/mercury_tags.h	2001/02/21 05:17:48
> >  
> > +#ifdef MR_RESERVE_TAG
> > +    #define MR_RAW_TAG_VAR               0     /* for Prolog-style variables */
> > +    #define MR_FIRST_UNRESERVED_RAW_TAG  1
> > +#else
> > +    #define MR_FIRST_UNRESERVED_RAW_TAG  0
> > +#endif
> 
> That should be indented two spaces rather than four.

Done.

> > +#define MR_RAW_TAG_CONS         MR_FIRST_UNRESERVED_RAW_TAG + 1
> 
> The bodies of macro definitions like that should be parenthesized.
> 
> > +++ runtime/mercury_type_info.h	2001/02/23 05:01:17
> > @@ -347,9 +347,22 @@
> >  ** Definitions for handwritten code, mostly for mercury_compare_typeinfo.
> >  */
> >  
> > -#define MR_COMPARE_EQUAL    0
> > -#define MR_COMPARE_LESS     1
> > -#define MR_COMPARE_GREATER  2
> > +#ifdef MR_RESERVE_TAG
> > +        /*
> > +        ** In reserve-tag grades, enumerations are disabled, so the
> > +        ** representation of the 'comparison_result' type is quite different.
> > +        ** The enumeration constants (for (<), (=) and (>)) wind up sharing 
> > +        ** the same primary tag (1), and are all allocated secondary tags
> > +        ** starting from 0.
> > +        */
> > +    #define MR_COMPARE_EQUAL    ((0 << LOW_TAG_BITS) + 1)
> > +    #define MR_COMPARE_LESS     ((1 << LOW_TAG_BITS) + 1)
> > +    #define MR_COMPARE_GREATER  ((2 << LOW_TAG_BITS) + 1)
> 
> You should write that using the MR_mkword(), MR_mktag(), and
> MR_mkbody() macros from runtime/mercury_tags.h, rather than via
> explicit bit shifting:
> 
>     #define MR_COMPARE_EQUAL    MR_mkword(MR_mktag(1), MR_mkbody(0))
>     #define MR_COMPARE_LESS     MR_mkword(MR_mktag(1), MR_mkbody(1))
>     #define MR_COMPARE_GREATER  MR_mkword(MR_mktag(1), MR_mkbody(2))
> 
> Or perhaps even avoiding the magic number 1:
> 
>     #define MR_COMPARE_TAG	MR_mktag(MR_FIRST_UNRESERVED_RAW_TAG)
> 
>     #define MR_COMPARE_EQUAL    MR_mkword(MR_COMPARE_TAG, MR_mkbody(0))
>     #define MR_COMPARE_LESS     MR_mkword(MR_COMPARE_TAG, MR_mkbody(1))
>     #define MR_COMPARE_GREATER  MR_mkword(MR_COMPARE_TAG, MR_mkbody(2))

I have changed it to the latter.

> ----------
> 
> The new `.rt' grade modifier should be documented in the help message
> for the `--grade' option in compiler/options.m.

Here's a new diff of compiler/options.m:

Index: options.m
===================================================================
RCS file: /home/staff/zs/imp/mercury/compiler/options.m,v
retrieving revision 1.313
diff -u -t -r1.313 options.m
--- options.m	2001/02/19 06:28:35	1.313
+++ options.m	2001/03/13 02:16:24
@@ -154,6 +154,7 @@
                 ;       stack_trace
                 ;       require_tracing
                 ;       use_trail
+                ;       reserve_tag
                 ;       use_minimal_model
                 ;       pic_reg
                 ;       tags
@@ -577,6 +578,7 @@
         require_tracing         -       bool(no),
         stack_trace             -       bool(no),
         use_trail               -       bool(no),
+        reserve_tag             -       bool(no),
         use_minimal_model       -       bool(no),
         pic_reg                 -       bool(no),
         tags                    -       string("low"),
@@ -995,6 +997,7 @@
 % long_option("stack-trace",           stack_trace).
 % long_option("require-tracing",       require_tracing).
 long_option("use-trail",                use_trail).
+long_option("reserve-tag",              reserve_tag).
 long_option("use-minimal-model",        use_minimal_model).
 long_option("pic-reg",                  pic_reg).
 long_option("tags",                     tags).
@@ -1936,8 +1939,8 @@
 % and the --gcc-nested-functions option is not yet documented.
 %               "\t`hl', `hl_nest', `hlc_nest'",
                 "\tor one of those with one or more of the grade modifiers",
-                "\t`.gc', `.prof', `.memprof', `.tr', `.debug', `.par', and/or",
-                "\t`.pic_reg' appended.",
+                "\t`.gc', `.prof', `.memprof', `.tr', `.rt', `.debug', `.par', 
+                "\tand/or `.pic_reg' appended.",
                 "\tDepending on your particular installation, only a subset",
                 "\tof these possible grades will have been installed.",
                 "\tAttempting to use a grade which has not been installed",
@@ -2055,6 +2058,12 @@
                 "\tThis is necessary for interfacing with constraint solvers,",
                 "\tor for backtrackable destructive update.",
                 "\tThis option is not yet supported for the IL or Java back-ends.",
+                "--reserve-tag\t\t\t(grade modifier: `.rt')",
+                "\tReserve a tag in the data representation of the generated ",
+                "\tcode. This tag is intended to be used to give an explicit",
+                "\trepresentation to free variables.",
+                "\tThis is necessary for a seamless Herbrand solver -",
+                "\tfor use with HAL.",
                 "-p, --profiling, --time-profiling",
                 "\t\t\t\t(grade modifier: `.prof')",
                 "\tEnable time and call profiling.  Insert profiling hooks in the",
> 
> The new option and grade modifier should be documented in
> doc/user_guide.texi too, as well as in the help message in
> compiler/options.m.

I have added this to the log message:
-----
doc/user_guide.texi:
        Document the `.rt' grade. 
-----
and made the following change:

Index: user_guide.texi
===================================================================
RCS file: /home/staff/zs/imp/mercury/doc/user_guide.texi,v
retrieving revision 1.241
diff -u -t -r1.241 user_guide.texi
--- user_guide.texi	2001/02/19 06:28:40	1.241
+++ user_guide.texi	2001/03/13 03:18:10
@@ -3446,6 +3446,9 @@
 @item Whether to enable the trail:
 @samp{tr} (the default is no trailing).
 
+ at item Whether or not to reserve a tag in the data representation of the generated code:
+ at samp{rt} (the default is no reserved tag)
+
 @item What debugging features to enable:
 @samp{debug} (the default is no debugging features).
 
@@ -3528,6 +3531,9 @@
 @item @samp{.tr}
 @code{--use-trail}.
 
+ at item @samp{.rt}
+ at code{--reserve-tag}.
+
 @item @samp{.debug}
 @code{--debug}.
 
@@ -3650,6 +3656,12 @@
 This is necessary for interfacing with constraint solvers,
 or for backtrackable destructive update.
 This option is not yet supported for the IL or Java back-ends.
+ at sp 1
+ at item @code{--reserve-tag} (grades: any grade containing @samp{.rt})
+Reserve a tag in the data representation of the generated 
+code. This tag is intended to be used to give an explicit
+representation to free variables.
+This is necessary for a seamless Herbrand solver - for use with HAL.
 @sp 1
 @item @code{--pic-reg} (grades: any grade containing `.pic_reg')
 [For Unix with intel x86 architecture only.]

> According to our C coding guidelines, the MR_define_univ_fields(),
> MR_unravel_univ(), and MR_new_univ_on_hp() macros should all be
> uppercase, since they're not function-like: they modify their arguments.
> But don't worry about that one, MR_define_univ_fields() was that way
> before your change.  We can fix that one as a separate change.
> 
> Apart from that, this looks fine.

If there are no objections, I will commit this in the next day or so, then.


dgj
-- 
David Jeffery (dgj at cs.mu.oz.au) | If you want to build a ship, don't drum up 
PhD student,                    | people together to collect wood or assign 
Dept. of Comp. Sci. & Soft. Eng.| them tasks and work, but rather teach them 
The University of Melbourne     | to long for the endless immensity of the sea.
Australia                       | -- Antoine de Saint Exupery
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------



More information about the developers mailing list