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

[Julien Fischer]

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.

> compiler/rbmm.region_instruction.m:
>        Define the new type for region instructions and modify the code to use
>        it.
>
> compiler/rbmm.region_transformation.m:
>        Modify the code to use the new type. Move the function that returns
>        builtin region type to prog_type.m.
>
> compiler/rbmm.condition_renaming.m
> compiler/rbmm.region_resurrection_renaming.m:
>        Change the code to use the new type and correct some formatting.
>
> compiler/rbmm.m:
>        Change a variable name and some alignment of source code.
>
> compiler/prog_type.m:
>        Add a function to return the builtin region type.

...

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

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

> +:- 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.

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.

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