[m-dev.] for review: fix MLDS static ground term bugs
Fergus Henderson
fjh at cs.mu.OZ.AU
Sun Oct 22 01:14:50 AEDT 2000
This change fixes several closely related bugs where the MLDS compiler
would generatic static ground terms whose initializers referred to
variables which weren't in scope. Unfortunately the fix isn't
yet complete: AFAIK it fixes all such problems in the MLDS generated
by the code generator, but the ml_elim_nested pass that follows still
screws things up.
Still, I might go ahead and commit this part soon anyway, since it
does at least solve this bug for the case when you compile with
--gcc-nested-functions, or when the bug arises in model_det/model_semi
code rather than in model_non code.
----------
Estimated hours taken: 8
Fix some bugs in the static ground term optimization for the MLDS
back-end.
compiler/ml_code_util.m:
compiler/ml_code_gen.m:
Ensure that declarations for a goal are generated in such a
way that they scope over the C code generated for any
following goals. This is needed to ensure that we don't
generate references to undeclared names in static constants.
compiler/ml_unify_gen.m:
compiler/ml_code_util.m:
Move ml_gen_static_const_decl_flags from ml_unify_gen.m
to ml_code_util.m, for use by ml_code_gen.m.
compiler/mark_static_terms.m:
Fix some typos in the comments.
tests/valid/Mmakefile:
tests/valid/static.m:
Add some regression tests.
Workspace: /home/pgrad/fjh/ws/hg
Index: compiler/mark_static_terms.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/mark_static_terms.m,v
retrieving revision 1.3
diff -u -d -r1.3 mark_static_terms.m
--- compiler/mark_static_terms.m 2000/10/06 10:18:25 1.3
+++ compiler/mark_static_terms.m 2000/10/21 11:41:00
@@ -67,16 +67,16 @@
conj_mark_static_terms(Goals0, Goals, SI0, SI).
goal_expr_mark_static_terms(disj(Goals0, B), disj(Goals, B), SI0, SI0) :-
- % we rever to the original static_info at the end of branched goals
+ % we revert to the original static_info at the end of branched goals
disj_mark_static_terms(Goals0, Goals, SI0).
goal_expr_mark_static_terms(switch(A, B, Cases0, D), switch(A, B, Cases, D),
SI0, SI0) :-
- % we rever to the original static_info at the end of branched goals
+ % we revert to the original static_info at the end of branched goals
cases_mark_static_terms(Cases0, Cases, SI0).
goal_expr_mark_static_terms(not(Goal0), not(Goal), SI0, SI0) :-
- % we rever to the original static_info at the end of the negation
+ % we revert to the original static_info at the end of the negation
goal_mark_static_terms(Goal0, Goal, SI0, _SI).
goal_expr_mark_static_terms(some(A, B, Goal0), some(A, B, Goal), SI0, SI) :-
Index: compiler/ml_code_gen.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/ml_code_gen.m,v
retrieving revision 1.60
diff -u -d -r1.60 ml_code_gen.m
--- compiler/ml_code_gen.m 2000/10/14 04:00:14 1.60
+++ compiler/ml_code_gen.m 2000/10/21 13:57:24
@@ -83,6 +83,25 @@
% means that in the situation described by [situation],
% for the the specified [construct] we will generate the specified [code].
+% There is one other important thing which can be considered part of the
+% calling convention for the code that we generate for each goal.
+% If static ground term optimization is enabled, then for the terms
+% marked as static by mark_static_terms.m, we will generate static consts.
+% These static consts can refer to other static consts defined earlier.
+% We need to be careful about the scopes of variables to ensure that
+% for any term that mark_static_terms.m marks as static, the C constants
+% representing the arguments of that term are in scope at the point
+% where that term is constructed. Basically this means that
+% all the static consts generated inside a goal must be hoist out to
+% the top level scope for that goal, except for goal types where
+% goal_expr_mark_static_terms (in mark_static_terms.m) returns the
+% same static_info unchanged, i.e. branched goals and negations.
+% For branched goals, we must in fact ensure that we do NOT hoist out
+% static variable definitions, since the same Mercury variable can have
+% different constant values in different branches, and so there may be
+% more than one C static const with the same name but with different
+% values and occurring in different scopes.
+
%-----------------------------------------------------------------------------%
%
% Code for wrapping goals
@@ -106,12 +125,10 @@
% semi goal in nondet context:
% <Goal && SUCCEED()>
% ===>
-% {
% bool succeeded;
%
% <succeeded = Goal>
% if (succeeded) SUCCEED();
-% }
%-----------------------------------------------------------------------------%
%
@@ -244,6 +261,10 @@
% <Goal && success()>
% }
+% Note that for all of these versions, we must hoist any static declarations
+% generated for <Goal> out to the top level; this is needed so that such
+% declarations remain in scope for any following goals.
+
%-----------------------------------------------------------------------------%
%
% Code for empty conjunctions (`true')
@@ -289,12 +310,12 @@
% <Goals>
% If the first goal is model_semidet, then there are two cases:
-% if the disj as a whole is semidet, things are simple, and
-% if the disj as a whole is model_non, then we do the same as
+% if the conj as a whole is semidet, things are simple, and
+% if the conj as a whole is model_non, then we do the same as
% for the semidet case, except that we also (ought to) declare
% a local `succeeded' variable.
%
-% model_semi Goal in model_semi disj:
+% model_semi Goal in model_semi conj:
% <succeeded = (Goal, Goals)>
% ===>
% <succeeded = Goal>;
@@ -302,17 +323,22 @@
% <Goals>;
% }
%
-% model_semi Goal in model_non disj:
+% model_semi Goal in model_non conj:
% <Goal && Goals>
% ===>
-% {
% bool succeeded;
%
% <succeeded = Goal>;
% if (succeeded) {
% <Goals>;
% }
-% }
+%
+% The actual code generation scheme we use is slightly
+% different to that: we hoist any declarations generated
+% for <Goals> to the outer scope, rather than keeping
+% them inside the `if', so that they remain in
+% scope for any later goals which follow this.
+% This is needed for declarations of static consts.
% For model_non goals, there are a couple of different
% ways that we could generate code, depending on whether
@@ -325,7 +351,6 @@
% model_non Goal (optimized for readability)
% <Goal, Goals>
% ===>
-% {
% entry_func() {
% <Goal && succ_func()>;
% }
@@ -334,7 +359,6 @@
% }
%
% entry_func();
-% }
%
% The more efficient method generates the goals in
% reverse order, so it's less readable, but it has fewer
@@ -344,13 +368,11 @@
% model_non Goal (optimized for efficiency):
% <Goal, Goals>
% ===>
-% {
% succ_func() {
% <Goals && SUCCEED()>;
% }
%
% <Goal && succ_func()>;
-% }
%
% The more efficient method is the one we actually use.
%
@@ -360,7 +382,6 @@
% model_non goals (optimized for readability):
% <Goal1, Goal2, Goal3, Goals>
% ===>
-% {
% label0_func() {
% <Goal1 && label1_func()>;
% }
@@ -375,12 +396,10 @@
% }
%
% label0_func();
-% }
%
% model_non goals (optimized for efficiency):
% <Goal1, Goal2, Goal3, Goals>
% ===>
-% {
% label1_func() {
% label2_func() {
% label3_func() {
@@ -391,7 +410,6 @@
% <Goal2 && label2_func()>;
% }
% <Goal1 && label1_func()>;
-% }
%
% Note that it might actually make more sense to generate
% conjunctions of nondet goals like this:
@@ -399,7 +417,6 @@
% model_non goals (optimized for efficiency, alternative version):
% <Goal1, Goal2, Goal3, Goals>
% ===>
-% {
% label3_func() {
% <Goals && SUCCEED()>;
% }
@@ -411,13 +428,21 @@
% }
%
% <Goal1 && label1_func()>;
-% }
%
% This would avoid the undesirable deep nesting that we sometimes get
% with our current scheme. However, if we're eliminating nested
% functions, as is normally the case, then after the ml_elim_nested
% transformation all the functions and variables have been hoisted
% to the top level, so there is no difference between these two.
+%
+% As with semidet conjunctions, we hoist declarations
+% out so that they remain in scope for any following goals.
+% This is needed for declarations of static consts.
+% However, we want to keep the declarations of non-static
+% variables local, since accessing local variables is more
+% efficient that accessing variables in the environment
+% from a nested function. So we only hoist declarations
+% of static constants.
%-----------------------------------------------------------------------------%
%
@@ -450,32 +475,27 @@
% model_semi Goal:
% <do (Goal ; Goals)>
% ===>
-% {
% bool succeeded;
%
% <succeeded = Goal>;
% if (!succeeded) {
% <do Goals>;
% }
-% }
% model_semi disj:
% model_det Goal:
% <succeeded = (Goal ; Goals)>
% ===>
-% {
% bool succeeded;
%
% <do Goal>
% succeeded = TRUE
% /* <Goals> will never be reached */
-% }
% model_semi Goal:
% <succeeded = (Goal ; Goals)>
% ===>
-% {
% bool succeeded;
%
% <succeeded = Goal>;
@@ -495,13 +515,11 @@
% model_semi Goal:
% <(Goal ; Goals) && SUCCEED()>
% ===>
-% {
% bool succeeded;
%
% <succeeded = Goal>
% if (succeeded) SUCCEED();
% <Goals && SUCCEED()>
-% }
%
% model_non Goal:
% <(Goal ; Goals) && SUCCEED()>
@@ -517,7 +535,6 @@
% model_semi Cond:
% <(Cond -> Then ; Else)>
% ===>
-% {
% bool succeeded;
%
% <succeeded = Cond>
@@ -526,7 +543,6 @@
% } else {
% <Else>
% }
-% }
% /*
% ** XXX The following transformation does not do as good a job of GC
@@ -538,7 +554,6 @@
% model_non Cond:
% <(Cond -> Then ; Else)>
% ===>
-% {
% bool cond_<N>;
%
% void then_func() {
@@ -551,7 +566,10 @@
% if (!cond_<N>) {
% <Else>
% }
-% }
+% except that we hoist any declarations generated
+% for <Cond> to the top of the scope, so that they
+% are in scope for the <Then> goal
+% (this is needed for declarations of static consts)
%-----------------------------------------------------------------------------%
%
@@ -561,12 +579,10 @@
% model_det negation
% <not(Goal)>
% ===>
-% {
% bool succeeded;
% <succeeded = Goal>
% /* now ignore the value of succeeded,
% which we know will be FALSE */
-% }
% model_semi negation, model_det Goal:
% <succeeded = not(Goal)>
@@ -1349,12 +1365,10 @@
% semi goal in nondet context:
% <Goal && SUCCEED()>
% ===>
- % {
% bool succeeded;
%
% <succeeded = Goal>
% if (succeeded) SUCCEED()
- % }
%
ml_gen_test_success(Succeeded),
ml_gen_call_current_success_cont(Context, CallCont),
@@ -1383,6 +1397,7 @@
ml_gen_commit(Goal, CodeModel, Context, MLDS_Decls, MLDS_Statements) -->
{ Goal = _ - GoalInfo },
{ goal_info_get_code_model(GoalInfo, GoalCodeModel) },
+ { goal_info_get_context(GoalInfo, GoalContext) },
( { GoalCodeModel = model_non, CodeModel = model_semi } ->
@@ -1434,7 +1449,13 @@
{ SuccessCont = success_cont(SuccessFuncLabelRval,
EnvPtrRval, [], []) },
ml_gen_info_push_success_cont(SuccessCont),
- ml_gen_goal(model_non, Goal, GoalStatement),
+ ml_gen_goal(model_non, Goal, GoalDecls, GoalStatements),
+ % hoist any static constant declarations for Goal
+ % out to the top level
+ { list__filter(ml_decl_is_static, GoalDecls, GoalStaticDecls,
+ GoalOtherDecls) },
+ { GoalStatement = ml_gen_block(GoalOtherDecls,
+ GoalStatements, GoalContext) },
ml_gen_info_pop_success_cont,
ml_gen_set_success(const(false), Context, SetSuccessFalse),
ml_gen_set_success(const(true), Context, SetSuccessTrue),
@@ -1446,7 +1467,8 @@
{ TryCommitStatement = mlds__statement(TryCommitStmt,
MLDS_Context) },
- { MLDS_Decls = [CommitRefDecl, SuccessFunc | LocalVarDecls] },
+ { MLDS_Decls = list__append([CommitRefDecl,
+ SuccessFunc | LocalVarDecls], GoalStaticDecls) },
{ MLDS_Statements = [TryCommitStatement] },
ml_gen_info_set_var_lvals(OrigVarLvalMap)
@@ -1496,7 +1518,13 @@
{ SuccessCont = success_cont(SuccessFuncLabelRval,
EnvPtrRval, [], []) },
ml_gen_info_push_success_cont(SuccessCont),
- ml_gen_goal(model_non, Goal, GoalStatement),
+ ml_gen_goal(model_non, Goal, GoalDecls, GoalStatements),
+ % hoist any static constant declarations for Goal
+ % out to the top level
+ { list__filter(ml_decl_is_static, GoalDecls, GoalStaticDecls,
+ GoalOtherDecls) },
+ { GoalStatement = ml_gen_block(GoalOtherDecls,
+ GoalStatements, GoalContext) },
ml_gen_info_pop_success_cont,
{ TryCommitStmt = try_commit(CommitRefLval, GoalStatement,
@@ -1504,7 +1532,8 @@
{ TryCommitStatement = mlds__statement(TryCommitStmt,
MLDS_Context) },
- { MLDS_Decls = [CommitRefDecl, SuccessFunc | LocalVarDecls] },
+ { MLDS_Decls = list__append([CommitRefDecl,
+ SuccessFunc | LocalVarDecls], GoalStaticDecls) },
{ MLDS_Statements = [TryCommitStatement] },
ml_gen_info_set_var_lvals(OrigVarLvalMap)
@@ -2506,7 +2535,6 @@
% model_semi cond:
% <(Cond -> Then ; Else)>
% ===>
- % {
% bool succeeded;
%
% <succeeded = Cond>
@@ -2515,7 +2543,6 @@
% } else {
% <Else>
% }
- % }
{ CondCodeModel = model_semi },
ml_gen_goal(model_semi, Cond, CondDecls, CondStatements),
ml_gen_test_success(Succeeded),
@@ -2541,7 +2568,6 @@
% model_non cond:
% <(Cond -> Then ; Else)>
% ===>
- % {
% bool cond_<N>;
%
% void then_func() {
@@ -2554,8 +2580,12 @@
% if (!cond_<N>) {
% <Else>
% }
- % }
+ % except that we hoist any declarations generated
+ % for <Cond> to the top of the scope, so that they
+ % are in scope for the <Then> goal
+ % (this is needed for declarations of static consts)
+
{ CondCodeModel = model_non },
% generate the `cond_<N>' var
@@ -2593,7 +2623,7 @@
{ IfStatement = mlds__statement(IfStmt, MLDS_Context) },
% package it all up in the right order
- { MLDS_Decls = [CondVarDecl, ThenFunc | CondDecls] },
+ { MLDS_Decls = list__append([CondVarDecl | CondDecls], [ThenFunc]) },
{ MLDS_Statements = list__append(
[SetCondFalse | CondStatements], [IfStatement]) }
).
Index: compiler/ml_code_util.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/ml_code_util.m,v
retrieving revision 1.23
diff -u -d -r1.23 ml_code_util.m
--- compiler/ml_code_util.m 2000/10/14 04:00:16 1.23
+++ compiler/ml_code_util.m 2000/10/21 13:40:22
@@ -241,6 +241,21 @@
%-----------------------------------------------------------------------------%
%
+% Routines for dealing with static constants
+%
+
+ % Return the declaration flags appropriate for an
+ % initialized local static constant.
+ %
+:- func ml_static_const_decl_flags = mlds__decl_flags.
+
+ % Succeed iff the specified mlds__defn defines
+ % a static constant.
+ %
+:- pred ml_decl_is_static(mlds__defn::in) is semidet.
+
+%-----------------------------------------------------------------------------%
+%
% Routines for dealing with fields
%
@@ -674,39 +689,51 @@
% model_semi goal:
% <Goal, Goals>
% ===>
- % {
% bool succeeded;
%
% <succeeded = Goal>;
% if (succeeded) {
% <Goals>;
% }
- % }
+ % except that we hoist any declarations generated
+ % for <Goals> to the outer scope, rather than
+ % inside the `if', so that they remain in
+ % scope for any later goals which follow this
+ % (this is needed for declarations of static consts)
{ FirstCodeModel = model_semi },
DoGenFirst(FirstDecls, FirstStatements),
ml_gen_test_success(Succeeded),
DoGenRest(RestDecls, RestStatements),
- { IfBody = ml_gen_block(RestDecls, RestStatements, Context) },
+ { IfBody = ml_gen_block([], RestStatements, Context) },
{ IfStmt = if_then_else(Succeeded, IfBody, no) },
{ IfStatement = mlds__statement(IfStmt,
mlds__make_context(Context)) },
- { MLDS_Decls = FirstDecls },
+ { MLDS_Decls = list__append(FirstDecls, RestDecls) },
{ MLDS_Statements = list__append(FirstStatements,
[IfStatement]) }
;
% model_non goal:
% <First, Rest>
% ===>
- % {
% succ_func() {
% <Rest && SUCCEED()>;
% }
%
% <First && succ_func()>;
- % }
+ % except that we hoist any declarations generated
+ % for <First> and any _static_ declarations generated
+ % for <Rest> to the top of the scope, rather
+ % than inside or after the succ_func(), so that they
+ % remain in scope for any code following them
+ % (this is needed for declarations of static consts).
%
- % XXX this leads to deep nesting for long conjunctions;
- % we should avoid that.
+ % We take care to only hoist _static_ declarations
+ % outside nested functions, since access to non-local
+ % variables is less efficient.
+ %
+ % XXX The pattern above leads to deep nesting for long
+ % conjunctions; we should avoid that.
+ %
{ FirstCodeModel = model_non },
@@ -714,7 +741,9 @@
ml_gen_new_func_label(no, RestFuncLabel, RestFuncLabelRval),
/* push nesting level */
DoGenRest(RestDecls, RestStatements),
- { RestStatement = ml_gen_block(RestDecls, RestStatements,
+ { list__filter(ml_decl_is_static, RestDecls, RestStaticDecls,
+ RestOtherDecls) },
+ { RestStatement = ml_gen_block(RestOtherDecls, RestStatements,
Context) },
/* pop nesting level */
ml_gen_nondet_label_func(RestFuncLabel, Context, RestStatement,
@@ -727,12 +756,19 @@
DoGenFirst(FirstDecls, FirstStatements),
ml_gen_info_pop_success_cont,
- % it might be better to put the decls in the other order:
- /* { MLDS_Decls = list__append(FirstDecls, [RestFunc]) }, */
- { MLDS_Decls = [RestFunc | FirstDecls] },
+ { MLDS_Decls = list__condense(
+ [FirstDecls, RestStaticDecls, [RestFunc]]) },
{ MLDS_Statements = FirstStatements }
).
+ % Succeed iff the specified mlds__defn defines
+ % a static constant.
+ %
+ml_decl_is_static(Defn) :-
+ Defn = mlds__defn(Name, _Context, Flags, _DefnBody),
+ Name = data(var(_)),
+ Flags = ml_static_const_decl_flags.
+
% Given a function label and the statement which will comprise
% the function body for that function, generate an mlds__defn
% which defines that function.
@@ -1211,6 +1247,7 @@
MLDS_Defn = mlds__defn(Name, Context, DeclFlags, Defn).
% Return the declaration flags appropriate for a local variable.
+ %
:- func ml_gen_var_decl_flags = mlds__decl_flags.
ml_gen_var_decl_flags = MLDS_DeclFlags :-
Access = public,
@@ -1218,6 +1255,19 @@
Virtuality = non_virtual,
Finality = overridable,
Constness = modifiable,
+ Abstractness = concrete,
+ MLDS_DeclFlags = init_decl_flags(Access, PerInstance,
+ Virtuality, Finality, Constness, Abstractness).
+
+ % Return the declaration flags appropriate for an
+ % initialized local static constant.
+ %
+ml_static_const_decl_flags = MLDS_DeclFlags :-
+ Access = private,
+ PerInstance = one_copy,
+ Virtuality = non_virtual,
+ Finality = overridable,
+ Constness = const,
Abstractness = concrete,
MLDS_DeclFlags = init_decl_flags(Access, PerInstance,
Virtuality, Finality, Constness, Abstractness).
Index: compiler/ml_unify_gen.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/ml_unify_gen.m,v
retrieving revision 1.19
diff -u -d -r1.19 ml_unify_gen.m
--- compiler/ml_unify_gen.m 2000/10/09 08:48:53 1.19
+++ compiler/ml_unify_gen.m 2000/10/21 12:32:34
@@ -1159,20 +1159,6 @@
MLDS_Context = mlds__make_context(Context),
MLDS_Defn = mlds__defn(Name, MLDS_Context, DeclFlags, Defn).
- % Return the declaration flags appropriate for an
- % initialized local static constant.
- %
-:- func ml_static_const_decl_flags = mlds__decl_flags.
-ml_static_const_decl_flags = MLDS_DeclFlags :-
- Access = private,
- PerInstance = one_copy,
- Virtuality = non_virtual,
- Finality = overridable,
- Constness = const,
- Abstractness = concrete,
- MLDS_DeclFlags = init_decl_flags(Access, PerInstance,
- Virtuality, Finality, Constness, Abstractness).
-
:- pred ml_cons_name(cons_id, ctor_name, ml_gen_info, ml_gen_info).
:- mode ml_cons_name(in, out, in, out) is det.
Index: tests/valid/Mmakefile
===================================================================
RCS file: /home/mercury1/repository/tests/valid/Mmakefile,v
retrieving revision 1.74
diff -u -d -r1.74 Mmakefile
--- tests/valid/Mmakefile 2000/10/13 13:56:17 1.74
+++ tests/valid/Mmakefile 2000/10/21 13:50:22
@@ -130,6 +130,7 @@
soln_context.m \
some_switch.m \
stack_alloc.m \
+ static.m \
subtype_switch.m \
switch_detection_bug.m \
switch_detection_bug2.m \
Index: tests/valid/static.m
===================================================================
RCS file: static.m
diff -N static.m
--- /dev/null Thu Mar 30 14:06:13 2000
+++ static.m Sun Oct 22 01:04:19 2000
@@ -0,0 +1,63 @@
+% mmc -c --grade hlc.gc static.c
+% static.c(455) : error C2065: 'static__const_Result_5' : undeclared identifier
+:- module static.
+
+:- interface.
+
+:- type t.
+:- type t4.
+:- type t5.
+
+:- pred q(t::in, t5::out) is det.
+:- pred r(t::in, t5::out, int::out) is multi.
+:- pred s(int::in, t5::out) is cc_nondet.
+
+:- implementation.
+:- import_module int.
+
+:- type t ---> a ; b ; c.
+:- type t4 ---> f(string, int).
+:- type t5 ---> g(t4, t4) ; i.
+
+% Test for ordinary if-then-else
+q(X, Y) :-
+ (
+ X = a,
+ % This line causes the problem. Move it into
+ % the then to avoid the above code gen problem.
+ % We can move it into the then because this line
+ % isn't part of the test.
+ Result = f("hello", 0)
+ ->
+ Y = g(Result, Result)
+ ;
+ Y = i
+ ).
+
+% Test for if-then-else with nondet condition
+r(X, Y, Z) :-
+ (
+ X = a,
+ (Z0 = 1 ; Z0 = 2),
+
+ % This line causes the problem. Move it into
+ % the then to avoid the above code gen problem.
+ % We can move it into the then because this line
+ % isn't part of the test.
+ Result = f("hello", 0)
+ ->
+ Z = Z0,
+ Y = g(Result, Result)
+ ;
+ Z = 0,
+ Y = i
+ ).
+
+% Test for commit
+s(X, Y) :-
+ some [Z] (
+ (Z = 1 ; Z = 2),
+ X = Z * Z,
+ Result = f("hello", 0)
+ ),
+ Y = g(Result, Result).
--
Fergus Henderson <fjh at cs.mu.oz.au> | "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh> | of excellence is a lethal habit"
| -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to: mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions: mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------
More information about the developers
mailing list