[m-rev.] For review: Use type for region instruction

[Quan Phan]

Quoting Julien Fischer <juliensf at csse.unimelb.edu.au>:

> 
> On Thu, 19 Jul 2007, Quan Phan wrote:
> 
> > Estimated hours taken: 2.
> > Branches: main.
> >
> > Defining and using a specific type for region instructions (instead of
> string).
> 
> I suggest:
> 
>  	Define and use a specific type to represent region instructions,
>  	rather than encoding them as strings.

I made the change.

> 
> > Index: rbmm.region_instruction.m
> > ===================================================================
> > RCS file:
> >
> /home/mercury/mercury1/repository/mercury/compiler/rbmm.region_instruction.m,v
> > retrieving revision 1.2
> > diff -u -u -r1.2 rbmm.region_instruction.m
> > --- rbmm.region_instruction.m	8 Jun 2007 06:45:11 -0000	1.2
> > +++ rbmm.region_instruction.m	19 Jul 2007 05:56:03 -0000
> > @@ -28,19 +28,51 @@
> > :- import_module map.
> > :- import_module string.
> >
> > -:- type annotation_table == map(pred_proc_id, annotation_proc).
> > +:- type region_instruction_table == map(pred_proc_id,
> region_instruction_proc).
> >
> > -:- type annotation_proc == map(program_point, before_after).
> > -:- type before_after ---> before_after(list(string), list(string)).
> > +:- type region_instruction_proc == map(program_point,
> insts_before_after).
> > +
> > +:- type insts_before_after
> 
> Add a comment describing the purpose of this type.
> 

Comment added.

> > +
> > +    --->    insts_before_after(
> > +                insts_before    :: list(region_instruction),
> > +                                % Region instructions before a program
> point.
> > +
> > +                insts_after     :: list(region_instruction)
> > +                                % Region instructions after a program
> point.
> > +            ).
> > +
> 
> Does inst here mean instruction?  I don't think it is a good name as
> `inst' in the context of Mercury usually means something quite
> different.
> 

Yes, so I change "inst" to "instruction".

> > +:- type region_instruction
> > +
> > +    --->    create_region(
> > +                % Region name.
> > +                string
> > +            )
> > +
> > +    ;       remove_region(
> > +                % Region name.
> > +                string
> > +            )
> > +
> > +    ;       rename_region(
> > +                % Old region name.
> > +                string,
> > +
> > +                % New region name.
> > +                string
> > +            ).
> > +
> > +:- type region_instruction_type
> > +
> > +    --->    create_region_inst
> > +    ;       remove_region_inst
> > +    ;       renaming_region_inst.
> 
> Again, avoid using `inst' in this context.
> 

They are also changed to instruction.


> I've glanced through the rest of it and  it seems okay.  We should
> reconsider the way that nodes in the region graph are identified;
> again using strings to do that seems quite error prone.
> 

I don't mind doing this. I think we should think about this with consideration
to the "renaming region" stuff because the renaming solution is currently based
on these strings.

Thanks and regards,
Quan

> Julien.
> --------------------------------------------------------------------------
> mercury-reviews mailing list
> Post messages to:       mercury-reviews at csse.unimelb.edu.au
> Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
> Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
> --------------------------------------------------------------------------
> 



--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------