[m-rev.] for review: [CTGC] indirect reuse analysis.
Nancy
Nancy.Mazur at cs.kuleuven.ac.be
Mon May 29 22:41:36 AEST 2006
Hi Julien,
>> Index: compiler/ctgc.livedata.m
...
>> +:- type livedata
>> + ---> bottom % There are no live data structures.
>> + ; top % All data structures may be live.
>> + ; live(list(datastruct)).
>> + % Only the listed datastructures can possibly
>> + % be live.
>
> There are already types with constructors called bottom and top in several parts
> of the compiler. It would probably a good idea to use more distinct names,
> e.g.
>
> :- type livedata
> ---> livedata_bottom
> ; livedata_top
> ; livedata_live(list(datastruct)).
>
> (It's probably worth doing a bit of renaming along similar lines in
> other parts of the CTGC system as well).
I've done the renaming for the private representation of structure
sharing. I'll do the changes to the prog_data types separately.
>> +livedata_project(ProgVars, LiveData) = ProjectedLiveData :-
>> + (
>> + LiveData = bottom,
>> + ProjectedLiveData = bottom
>> + ;
>> + LiveData = top,
>> + ProjectedLiveData = top
>> + ;
>> + LiveData = live(Data),
>> + list.filter(list_contains_datastruct_var(ProgVars),
>> + Data, FilteredData),
>> + ( FilteredData = [] -> ProjectedLiveData = bottom
>> + ; ProjectedLiveData = live(FilteredData)
>> + )
>
> That if-then-else can be a switch.
ok
>> Index: compiler/ctgc.util.m
>> ===================================================================
>> RCS file: /home/mercury1/repository/mercury/compiler/ctgc.util.m,v
>> retrieving revision 1.5
>> diff -u -d -r1.5 ctgc.util.m
>> --- compiler/ctgc.util.m 10 May 2006 10:56:49 -0000 1.5
>> +++ compiler/ctgc.util.m 18 May 2006 12:01:05 -0000
>> @@ -18,8 +18,10 @@
>>
>> :- import_module hlds.hlds_module.
>> :- import_module hlds.hlds_pred.
>> +:- import_module parse_tree.prog_data.
>>
>> :- import_module list.
>> +:- import_module map.
>>
>> %-----------------------------------------------------------------------------%
>>
>> @@ -34,16 +36,32 @@
>> :- pred pred_requires_no_analysis(module_info::in, pred_id::in) is semidet.
>> :- pred pred_requires_analysis(module_info::in, pred_id::in) is semidet.
>>
>> + % Given the pred_proc_id of a procedure call and its actual arguments,
>> + % determine the variable renaming to rename anything which is defined
>> + % in terms of the formal arguments of the called procedure to the context
>> + % of the actual arguments.
>> + %
>> +:- func get_variable_renaming(module_info, pred_proc_id, prog_vars) =
>> + map(prog_var, prog_var).
>> +
>
> The module goal_util defines the type prog_var_renaming (==map(prog_var,
> prog_var)).
Thanks for that tip! I wouldn't have known otherwise!
>> Index: compiler/structure_reuse.direct.detect_garbage.m
>> ===================================================================
>> RCS file: /home/mercury1/repository/mercury/compiler/structure_reuse.direct.detect_garbage.m,v
>> retrieving revision 1.1
>> diff -u -d -r1.1 structure_reuse.direct.detect_garbage.m
>> --- compiler/structure_reuse.direct.detect_garbage.m 10 May 2006 10:56:56 -0000 1.1
>> +++ compiler/structure_reuse.direct.detect_garbage.m 18 May 2006 12:01:12 -0000
>> @@ -90,6 +90,7 @@
>> func(C) = G :- (G = C ^ case_goal), Cases), !SharingAs,
>> !DeadCellTable)
>> ;
>> + % XXX
>
> Why has that XXX comment been added there?
I'm planning to explicitly recheck all the handling of the negations...
As I need to recheck that with the theory.
I've specified the reason for the XXX now.
>
>> GoalExpr = not(_Goal)
>> ;
>> GoalExpr = scope(_, SubGoal),
>> @@ -208,28 +209,6 @@
>>
>> var_is_live(Var, LiveData) :-
>> list.member(datastruct_init(Var), LiveData).
>
> ...
>
>> + % reuse_as_rename_using_module_info(ModuleInfo, PPId,
>> + % ActualVars, ActualTypes, ActualTVarset, FormalReuse, ActualReuse):
>> + %
>> + % Renaming of the formal description of structure reuse conditions to the
>> + % actual description of these conditions. The information about the formal
>> + % variables needs to be extracted from the module information.
>> + % A list of variables and types is used as the actual variables and types.
>> + % The type variables set in the actual context must also be specified.
>> + %
>
> I don't understand the last part of that description.
I've changed this to:
% The actual names are determined by the actual variables names, the
% actual types, and the type-variables occurring in those types.
%
>
>> +:- pred reuse_as_rename_using_module_info(module_info::in,
>> + pred_proc_id::in, prog_vars::in, list(mer_type)::in, tvarset::in,
>> + reuse_as::in, reuse_as::out) is det.
>> +
>> % Given a variable and type variable mapping, rename the reuses
>> % conditions accordingly.
>
> ...
>
>> +:- func reuse_condition_from_called_proc_to_local_condition(module_info,
>> + proc_info, prog_vars, live_datastructs, sharing_as, reuse_condition) =
>> + reuse_condition.
>> +
>> +reuse_condition_from_called_proc_to_local_condition(ModuleInfo, ProcInfo,
>> + HeadVars, LuData, SharingAs, CalledCondition) = LocalCondition :-
>> + (
>> + CalledCondition = always, % should not occur though...
>> + LocalCondition = always
>
> If it shouldn't occur then why not call unexpected/2 here?
you are right.
>
>> + ;
>> + CalledCondition = condition(CalledDeadNodes,
>> + CalledInUseNodes, CalledSharingAs),
>> +
>> + % Translate the dead nodes:
>> + AllDeadNodes = extend_datastructs(ModuleInfo, ProcInfo,
>> + SharingAs, CalledDeadNodes),
>> + AllDeadHeadVarNodes = datastructs_project(HeadVars, AllDeadNodes),
>> +
>> + (
>> + AllDeadHeadVarNodes = []
>> + ->
>> + LocalCondition = always
>> + ;
>> + % Translate the in use nodes:
>> + AllInUseNodes = extend_datastructs(ModuleInfo, ProcInfo,
>> + SharingAs, list.append(LuData, CalledInUseNodes)),
>> + AllInUseHeadVarNodes = datastructs_project(HeadVars,
>> + AllInUseNodes),
>> +
>> + % Translate the sharing information:
>> + AllLocalSharing = sharing_as_comb(ModuleInfo, ProcInfo,
>> + CalledSharingAs, SharingAs),
>> + AllHeadVarLocalSharing = sharing_as_project(HeadVars,
>> + AllLocalSharing),
>> +
>> + LocalCondition = condition(AllDeadHeadVarNodes,
>> + AllInUseHeadVarNodes, AllHeadVarLocalSharing)
>> + )
>
> You can turn that if-then-else into a switch.
ok
>> Index: compiler/structure_reuse.indirect.m
...
>> + % The type analysis_info gathers the analysis information that may change
>> + % from goal to goal.
>> + %
>> +:- type analysis_info
>> + ---> analysis_info(
>> + sharing_as :: sharing_as,
>> + reuse_as :: reuse_as,
>> + static_vars :: set(prog_var),
>> + fptable :: sr_fixpoint_table
>> + ).
>> +
>
> analysis_info is not a good name for this as it's also the name of one of
> the principle data structures used by the intermodule-analysis framework
> (which we will eventually be using here.)
I've changed this to the name "ir_analysis_info" (ir standing for
indirect reuse).
....
>> +verify_indirect_reuse_2(BaseInfo, AnalysisInfo, GoalInfo, CalleePPId,
>> + CalleeArgs, FormalReuseAs, NewReuseAs):-
>> + ModuleInfo = BaseInfo ^ module_info,
>> + PredInfo = BaseInfo ^ pred_info,
>> + ProcInfo = BaseInfo ^ proc_info,
>> + SharingAs = AnalysisInfo ^ sharing_as,
>> + proc_info_get_vartypes(ProcInfo, ActualVarTypes),
>> + pred_info_get_typevarset(PredInfo, ActualTVarset),
>> + list.map(map.lookup(ActualVarTypes), CalleeArgs, CalleeTypes),
>> + reuse_as_rename_using_module_info(ModuleInfo, CalleePPId,
>> + CalleeArgs, CalleeTypes, ActualTVarset, FormalReuseAs, ActualReuseAs),
>> + LiveData = livedata_init_at_goal(ModuleInfo, ProcInfo, GoalInfo,
>> + SharingAs),
>> + ProjectedLiveData = livedata_project(CalleeArgs, LiveData),
>> + (
>> + % Given the preconditions, livedata wil not be empty, and
>
> s/wil/will/
>
> I suggest adding a sanity checking that verifys those preconditions.
Actually, that comment was unneeded. The checks are explicitly present
in the called procedure, i.e. reuse_as_satisfied/..
> ...
>
I'll commit this part...
Thanks for the review!
Nancy
Disclaimer: http://www.kuleuven.be/cwis/email_disclaimer.htm
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------
More information about the reviews
mailing list