[m-rev.] fix review: fix bug with ctgc and lcmc in high-level C grades

[Julien Fischer]

On Mon, 4 Feb 2008, Peter Wang wrote:

> Estimated hours taken: 4
> Branches: main
>
> Fix a bug with structure reuse and the last-call-modulo-cons optimisation on
> MLDS backends.  When constructing a cell by reusing a dead cell, we didn't
> take into account that the lcmc optimisation needs to assign the address of
> some fields into variables.  Those variables remained unassigned and pointed
> into random memory locations.
>
> Only tested on hlc grades with low-level data.

LCMC is not currently compatible with --high-level-data or the non-C
backends.

> compiler/ml_unify_gen.m:
> 	Fix the bug by generating statements to take the address of reused
> 	cell fields when appropriate.
>
> Index: compiler/ml_unify_gen.m
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/compiler/ml_unify_gen.m,v
> retrieving revision 1.122
> diff -u -r1.122 ml_unify_gen.m
> --- compiler/ml_unify_gen.m	21 Jan 2008 00:32:52 -0000	1.122
> +++ compiler/ml_unify_gen.m	4 Feb 2008 02:01:31 -0000
> @@ -686,8 +686,8 @@
>         Statement = statement(Stmt, mlds_make_context(Context)),
>
>         ml_gen_field_take_address_assigns(TakeAddrInfos, VarLval, MLDS_Type,
> -            MaybeTag, Context, !.Info, FieldAssigns),
> -        Statements = [Statement | FieldAssigns],
> +            MaybeTag, Context, !.Info, TakeAddrStatements),
> +        Statements = [Statement | TakeAddrStatements],
>         Decls = []
>     ;
>         HowToConstruct = construct_statically(StaticArgs),
> @@ -810,7 +810,7 @@
>
>         ml_gen_type(!.Info, Type, MLDS_DestType),
>         CastVar2Rval = unop(cast(MLDS_DestType), Var2Rval),
> -        Statement = ml_gen_assign(Var1Lval, CastVar2Rval, Context),
> +        AssignStatement = ml_gen_assign(Var1Lval, CastVar2Rval, Context),
>
>         % For each field in the construction unification we need to generate
>         % an rval.  ExtraRvals need to be inserted at the start of the object.
> @@ -818,11 +818,14 @@
>             0, ConsIdTag, Context, ExtraRvalStatements, !Info),
>         % XXX we do more work than we need to here, as some of the cells
>         % may already contain the correct values.
> -        ml_gen_unify_args(ConsId, ArgVars, ArgModes, ArgTypes, Fields, Type,
> -            VarLval, OffSet, ArgNum, ConsIdTag, Context, Statements0, !Info),
> -
> -        Decls = [],
> -        Statements = [Statement | ExtraRvalStatements] ++ Statements0
> +        ml_gen_unify_args_for_reuse(ConsId, ArgVars, ArgModes, ArgTypes,
> +            Fields, TakeAddr, Type, VarLval, OffSet, ArgNum, ConsIdTag,
> +            Context, FieldStatements, TakeAddrInfos, !Info),
> +        ml_gen_field_take_address_assigns(TakeAddrInfos, VarLval, MLDS_Type,
> +            MaybeTag, Context, !.Info, TakeAddrStatements),
> +        Statements = [AssignStatement | ExtraRvalStatements] ++
> +            FieldStatements ++ TakeAddrStatements,
> +        Decls = []
>     ;
>         HowToConstruct = construct_in_region(_RegVar),
>         sorry(this_file, "ml_gen_new_object: " ++
> @@ -1557,6 +1560,59 @@
>     ml_gen_unify_arg(ConsId, Arg, Mode, ArgType, Field, VarType, VarLval,
>         Offset, ArgNum, Tag, Context, !Statements, !Info).
>
> +:- pred ml_gen_unify_args_for_reuse(cons_id::in, prog_vars::in,
> +    list(uni_mode)::in, list(mer_type)::in, list(constructor_arg)::in,
> +    list(int)::in, mer_type::in, mlds_lval::in, int::in, int::in, cons_tag::in,
> +    prog_context::in, statements::out, list(take_addr_info)::out,
> +    ml_gen_info::in, ml_gen_info::out) is det.
> +
> +ml_gen_unify_args_for_reuse(ConsId, Args, Modes, ArgTypes, Fields, TakeAddr,
> +        VarType, VarLval, Offset, ArgNum, Tag, Context,
> +        Statements, TakeAddrInfos, !Info) :-
> +    (
> +        Args = [],
> +        Modes = [],
> +        ArgTypes = [],
> +        Fields = []
> +    ->
> +        Statements = [],
> +        TakeAddrInfos = []
> +    ;
> +        Args = [Arg | Args1],
> +        Modes = [Mode | Modes1],
> +        ArgTypes = [ArgType | ArgTypes1],
> +        Fields = [Field | Fields1]
> +    ->
> +        Offset1 = Offset + 1,
> +        ArgNum1 = ArgNum + 1,
> +        ( TakeAddr = [ArgNum | TakeAddr1] /* ? */ ->

Why is there a question mark there?

> +            ml_gen_unify_args_for_reuse(ConsId, Args1, Modes1, ArgTypes1,
> +                Fields1, TakeAddr1, VarType, VarLval, Offset1, ArgNum1,
> +                Tag, Context, Statements, TakeAddrInfos0, !Info),
> +
> +            FieldType = Field ^ arg_type,
> +            ml_gen_info_get_module_info(!.Info, ModuleInfo),
> +            module_info_get_globals(ModuleInfo, Globals),
> +            globals.lookup_bool_option(Globals, highlevel_data, HighLevelData),
> +            ml_type_as_field(FieldType, ModuleInfo, HighLevelData,
> +                BoxedFieldType),
> +            ml_gen_type(!.Info, FieldType, MLDS_FieldType),
> +            ml_gen_type(!.Info, BoxedFieldType, MLDS_BoxedFieldType),
> +            TakeAddrInfo = take_addr_info(Arg, Offset, MLDS_FieldType,
> +                MLDS_BoxedFieldType),
> +            TakeAddrInfos = [TakeAddrInfo | TakeAddrInfos0]
> +        ;
> +            ml_gen_unify_args_for_reuse(ConsId, Args1, Modes1, ArgTypes1,
> +                Fields1, TakeAddr, VarType, VarLval, Offset1, ArgNum1,
> +                Tag, Context, Statements0, TakeAddrInfos, !Info),
> +            ml_gen_unify_arg(ConsId, Arg, Mode, ArgType, Field,
> +                VarType, VarLval, Offset, ArgNum, Tag, Context,
> +                Statements0, Statements, !Info)

That looks okay otherwise.

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