Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Optimize true/false conditions when coding if-statements #2199

Merged
merged 1 commit into from
Feb 27, 2018
Merged

Optimize true/false conditions when coding if-statements #2199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions lib/system.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,6 @@ fi;
## For HPC-GAP, we want GAPInfo and its members to be accessible from all
## threads, so make members atomic or immutable.
##
if not IsHPCGAP then
# HACK to silence "Unbound global variable" warnings
AtomicList := fail;
AtomicRecord := fail;
fi;
if IsHPCGAP then
MakeReadWriteGVar("GAPInfo");
GAPInfo := AtomicRecord(GAPInfo);
Expand Down Expand Up @@ -475,13 +470,6 @@ CallAndInstallPostRestore( function()
end );


if not IsHPCGAP then
# undo HACK to silence "Unbound global variable" warnings
Unbind(AtomicList);
Unbind(AtomicRecord);
fi;


#############################################################################
##
#V GAPInfo.TestData
Expand Down
81 changes: 37 additions & 44 deletions src/code.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -921,13 +921,34 @@ void CodeIfElse ( void )

void CodeIfBeginBody ( void )
{
// get and check the condition
Expr cond = PopExpr();

// if the condition is 'false', ignore the body
if (TNUM_EXPR(cond) == T_FALSE_EXPR) {
STATE(IntrIgnoring) = 1;
}
else {
// put the condition expression back on the stack
PushExpr(cond);
}
}

void CodeIfEndBody (
Int CodeIfEndBody (
UInt nr )
{
/* collect the statements in a statement sequence if necessary */
PushStat( PopSeqStat( nr ) );

// get and check the condition
Expr cond = PopExpr();
PushExpr(cond);

// if the condition is 'true', ignore other branches of the if-statement
if (TNUM_EXPR(cond) == T_TRUE_EXPR) {
STATE(IntrIgnoring) = 1;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I don't like about this PR is that I have to access IntrIgnoring twice in src/code.c; previously, there was no other access to IntrIgnoring, IntrReturning or IntrCoding in src/code.c. And only a handful of access to any of them outside of src/intrprt.c (namely in src/gap.c and src/read.c).

But this is only a minor blemish, IMHO

}
return 1;
}

void CodeIfEnd (
Expand All @@ -937,51 +958,23 @@ void CodeIfEnd (
Expr cond; /* condition of a branch */
UInt hase; /* has else branch */
UInt i; /* loop variable */
Expr cond1 = 0; /* first condition */
Expr cond2 = 0; /* second condition */

/* peek at the last two conditions */
cond1 = PopExpr();
hase = (TNUM_EXPR(cond1) == T_TRUE_EXPR);
if (nr == 2) {
cond2 = PopExpr();
PushExpr(cond2);
}
PushExpr(cond1);

// Some optimisation cases
if (nr == 1) {
if (TNUM_EXPR(cond1) == T_TRUE_EXPR) {
// Leave statement
PopExpr();
return;
}
else if (TNUM_EXPR(cond1) == T_FALSE_EXPR) {
// Remove entire if statement
PopStat();
PopExpr();
PushStat(NewStat(T_EMPTY, 0));
return;
}

// if all conditions were false, the if-statement is an empty statement
if (nr == 0) {
PushStat(NewStat(T_EMPTY, 0));
return;
}

if (nr == 2 && hase) {
if (TNUM_EXPR(cond2) == T_TRUE_EXPR) {
// Leave 'true' case
PopStat();
PopExpr();
PopExpr();
return;
}
else if (TNUM_EXPR(cond2) == T_FALSE_EXPR) {
// Leave 'false' case
Stat body = PopStat();
PopExpr();
PopStat();
PopExpr();
PushStat(body);
return;
}
// peek at the last condition
cond = PopExpr();
hase = (TNUM_EXPR(cond) == T_TRUE_EXPR);
PushExpr(cond);

// optimize 'if true then BODY; fi;' to just 'BODY;'
if (nr == 1 && hase) {
// drop the condition expression, leave the body statement
PopExpr();
return;
}

/* allocate the if-statement */
Expand Down
2 changes: 1 addition & 1 deletion src/code.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ extern void CodeIfElse ( void );

extern void CodeIfBeginBody ( void );

extern void CodeIfEndBody (
extern Int CodeIfEndBody (
UInt nr );

extern void CodeIfEnd (
Expand Down
36 changes: 17 additions & 19 deletions src/intrprtr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,13 @@ void IntrIfBegin ( void )
{
/* ignore or code */
if ( STATE(IntrReturning) > 0 ) { return; }

// if IntrIgnoring is positive, increment it, as IntrIgnoring == 1 has a
// special meaning when parsing if-statements -- it is used to skip
// interpreting or coding branches of the if-statement which never will be
// executed, either because a previous branch is always executed (i.e., it
// has a 'true' condition), or else because the current branch has a
// 'false' condition
if ( STATE(IntrIgnoring) > 0 ) { STATE(IntrIgnoring)++; return; }
if ( STATE(IntrCoding) > 0 ) { CodeIfBegin(); return; }

Expand Down Expand Up @@ -597,22 +604,15 @@ void IntrIfBeginBody ( void )
}
}

void IntrIfEndBody (
Int IntrIfEndBody (
UInt nr )
{
UInt i; /* loop variable */

/* ignore or code */
if ( STATE(IntrReturning) > 0 ) { return; }
if ( STATE(IntrIgnoring) > 1 ) { STATE(IntrIgnoring)--; return; }
if ( STATE(IntrCoding) > 0 ) { CodeIfEndBody( nr ); return; }


/* if the condition was 'false', the body was ignored */
if ( STATE(IntrIgnoring) == 1 ) {
STATE(IntrIgnoring) = 0;
return;
}
if ( STATE(IntrReturning) > 0 ) { return 0; }
if ( STATE(IntrIgnoring) > 0 ) { STATE(IntrIgnoring)--; return 0; }
if ( STATE(IntrCoding) > 0 ) { return CodeIfEndBody( nr ); }

/* otherwise drop the values for the statements executed in the body */
for ( i = nr; 1 <= i; i-- ) {
Expand All @@ -621,6 +621,8 @@ void IntrIfEndBody (

/* one branch of the if-statement was executed, ignore the others */
STATE(IntrIgnoring) = 1;

return 1;
}

void IntrIfEnd (
Expand All @@ -629,19 +631,15 @@ void IntrIfEnd (
/* ignore or code */
if ( STATE(IntrReturning) > 0 ) { return; }
if ( STATE(IntrIgnoring) > 1 ) { STATE(IntrIgnoring)--; return; }
if ( STATE(IntrCoding) > 0 ) { CodeIfEnd( nr ); return; }


/* if one branch was executed (ignoring the others) */
// if one branch was executed (ignoring the others), reset IntrIgnoring
if ( STATE(IntrIgnoring) == 1 ) {
STATE(IntrIgnoring) = 0;
PushVoidObj();
}

/* if no branch was executed */
else {
PushVoidObj();
}
if ( STATE(IntrCoding) > 0 ) { CodeIfEnd( nr ); return; }

PushVoidObj();
}


Expand Down
2 changes: 1 addition & 1 deletion src/intrprtr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ extern void IntrIfElse ( void );

extern void IntrIfBeginBody ( void );

extern void IntrIfEndBody (
extern Int IntrIfEndBody (
UInt nr );

extern void IntrIfEnd (
Expand Down
9 changes: 3 additions & 6 deletions src/read.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2124,8 +2124,7 @@ void ReadIf (
Match( S_THEN, "then", STATBEGIN|S_ELIF|S_ELSE|S_FI|follow );
TRY_READ { IntrIfBeginBody(); }
nrs = ReadStats( S_ELIF|S_ELSE|S_FI|follow );
TRY_READ { IntrIfEndBody( nrs ); }
nrb++;
TRY_READ { nrb += IntrIfEndBody( nrs ); }

/* { 'elif' <Expr> 'then' <Statments> } */
while ( STATE(Symbol) == S_ELIF ) {
Expand All @@ -2135,8 +2134,7 @@ void ReadIf (
Match( S_THEN, "then", STATBEGIN|S_ELIF|S_ELSE|S_FI|follow );
TRY_READ { IntrIfBeginBody(); }
nrs = ReadStats( S_ELIF|S_ELSE|S_FI|follow );
TRY_READ { IntrIfEndBody( nrs ); }
nrb++;
TRY_READ { nrb += IntrIfEndBody( nrs ); }
}

/* [ 'else' <Statments> ] */
Expand All @@ -2145,8 +2143,7 @@ void ReadIf (
Match( S_ELSE, "else", follow );
TRY_READ { IntrIfBeginBody(); }
nrs = ReadStats( S_FI|follow );
TRY_READ { IntrIfEndBody( nrs ); }
nrb++;
TRY_READ { nrb += IntrIfEndBody( nrs ); }
}

/* 'fi' */
Expand Down
24 changes: 0 additions & 24 deletions tst/testbugfix/2017-09-07-elseif.tst
View file
Open in desktop

This file was deleted.

Loading