[m-rev.] for review: fix warnings on x86_64

[Mark Brown]

On 31-Aug-2005, Ian MacLarty <maclarty at cs.mu.OZ.AU> wrote:
> On Wed, 31 Aug 2005, Mark Brown wrote:
> 
> > Hi,
> >
> > Most of this is straightforward.  The main question is what to do about
> > the XXX comment in runtime/mercury_types.h.  I'm not sure what should be
> > done here, or even if this is a problem at all.
> >
> 
> Which comment do you mean?  I don't see an XXX in mercury_types.h in the
> diff.

Sorry.  It's the one in mercury_proc_id.h which you pick out below.

> 
> > Index: runtime/mercury_proc_id.h
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/runtime/mercury_proc_id.h,v
> > retrieving revision 1.4
> > diff -u -r1.4 mercury_proc_id.h
> > --- runtime/mercury_proc_id.h	23 May 2004 22:16:51 -0000	1.4
> > +++ runtime/mercury_proc_id.h	30 Aug 2005 14:06:54 -0000
> > @@ -15,7 +15,10 @@
> >  /*
> >  ** This type indicates whether a procedure came from a predicate or a function.
> >  ** This enum should EXACTLY match the definition of the `pred_or_func' type
> > -** in browser/util.m.
> > +** in mdbcomp/prim_data.m.
> > +**
> > +** XXX enums have type int, which is 32 bits on x86_64, whereas the
> > +** pred_or_func type is 64 bits.
> >  */
> >
> 
> Is this likely to cause any problems?  If you assign the enum to a 64 bit
> word will the higher bits be zeroed out?  If you compare a small 32-bit value
> with the same value in 64-bits will they compare as equal?

Either the type of MR_PredFunc needs to be changed, or the comment in
mdbcomp/prim_data.m that says "the code (in browser) assumes the
representation is the same" should be changed, since the representation
is not the same.  In any case, the meaning of "EXACTLY" should probably be
clarified.

Note that if we ever export a Mercury predicate with a pred_or_func output
(I don't know if we currently do) then we would run into problems if it is
passed an address of type MR_PredFunc*, since the predicate will overwrite
the 4 bytes after the MR_PredFunc location.

> 
> >  typedef	enum {
> > @@ -24,12 +27,15 @@
> >  } MR_PredFunc;
> >
> >  /*
> > -** MR_Proc_Id is a union. The usual alternative identifies ordinary
> > -** procedures, while the other alternative identifies automatically generated
> > -** unification, comparison and index procedures. The meanings of the fields
> > -** in both forms are the same as in procedure labels. The runtime system
> > -** can figure out which form is present by using the macro MR_PROC_ID_IS_UCI,
> > -** which will return true only if the procedure is of the second type.
> > +** MR_Proc_Id is a union. The first alternative identifies ordinary
> > +** procedures, while the second alternative identifies automatically generated
> > +** unification, comparison and index procedures.  The third alternative
> 
> Should initialise precedures be included in this list?

I don't think so.

> Otherwise that looks fine, though could you please bootcheck in the decldebug
> grade since it makes use of the fact that MR_PredFunc should be the same
> as the Mercury type.

It passed bootcheck in asm_fast.gc.tr.debug on saturn, which also makes use
of that fact, so that should be okay.  I'll bottcheck in decldebug before
committing though, just to be sure.

Cheers,
Mark.

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