Always check the execution mask after break/continue/return.

When "break", "continue", or "return" is used under varying control flow,
we now always check the execution mask to see if all of the program
instances are executing it.  (Previously, this was only done with "cbreak",
"ccontinue", and "creturn", which are now deprecated.)

An important effect of this change is that it fixes a family of cases
where we could end up running with an "all off" execution mask, which isn't
supposed to happen, as it leads to all sorts of invalid behavior.

This change does cause the volume rendering example to run 9% slower, but
doesn't affect the other examples.

Issue #257.

ctx.cpp

@@ -706,9 +706,6 @@ FunctionEmitContext::Break(bool doCoherenceCheck) {
     // jump to the break location.
     if (inSwitchStatement() == false && ifsInCFAllUniform(CFInfo::Loop)) {
         BranchInst(breakTarget);
-        if (ifsInCFAllUniform(CFInfo::Loop) && doCoherenceCheck)
-            Warning(currentPos, "Coherent break statement not necessary in "
-                    "fully uniform control flow.");
         // Set bblock to NULL since the jump has terminated the basic block
         bblock = NULL;
     }
@@ -778,9 +775,6 @@ FunctionEmitContext::Continue(bool doCoherenceCheck) {
         // which case we know that only a single program instance is
         // executing.
         AddInstrumentationPoint("continue: uniform CF, jumped");
-        if (doCoherenceCheck)
-            Warning(currentPos, "Coherent continue statement not necessary in "
-                    "fully uniform control flow.");
         BranchInst(continueTarget);
         bblock = NULL;
     }

ispc.h

@@ -454,7 +454,6 @@ struct Globals {
 
 enum {
     COST_ASSIGN = 1,
-    COST_COHERENT_BREAK_CONTINE = 4,
     COST_COMPLEX_ARITH_OP = 4,
     COST_DELETE = 32,
     COST_DEREF = 4,
@@ -465,7 +464,7 @@ enum {
     COST_GOTO = 4,
     COST_LOAD = 2,
     COST_NEW = 32,
-    COST_REGULAR_BREAK_CONTINUE = 2,
+    COST_BREAK_CONTINUE = 3,
     COST_RETURN = 4,
     COST_SELECT = 4,
     COST_SIMPLE_ARITH_LOGIC_OP = 1,

lex.ll

@@ -63,9 +63,9 @@ inline int isatty(int) { return 0; }
 #endif // ISPC_IS_WINDOWS
 
 static int allTokens[] = { 
-  TOKEN_ASSERT, TOKEN_BOOL, TOKEN_BREAK, TOKEN_CASE, TOKEN_CBREAK,
+  TOKEN_ASSERT, TOKEN_BOOL, TOKEN_BREAK, TOKEN_CASE,
-  TOKEN_CCONTINUE, TOKEN_CDO, TOKEN_CFOR, TOKEN_CIF, TOKEN_CWHILE,
+  TOKEN_CDO, TOKEN_CFOR, TOKEN_CIF, TOKEN_CWHILE,
-  TOKEN_CONST, TOKEN_CONTINUE, TOKEN_CRETURN, TOKEN_DEFAULT, TOKEN_DO,
+  TOKEN_CONST, TOKEN_CONTINUE, TOKEN_DEFAULT, TOKEN_DO,
   TOKEN_DELETE, TOKEN_DOUBLE, TOKEN_ELSE, TOKEN_ENUM,
   TOKEN_EXPORT, TOKEN_EXTERN, TOKEN_FALSE, TOKEN_FLOAT, TOKEN_FOR,
   TOKEN_FOREACH, TOKEN_FOREACH_ACTIVE, TOKEN_FOREACH_TILED,
@@ -96,15 +96,12 @@ void ParserInit() {
     tokenToName[TOKEN_BOOL] = "bool";
     tokenToName[TOKEN_BREAK] = "break";
     tokenToName[TOKEN_CASE] = "case";
-    tokenToName[TOKEN_CBREAK] = "cbreak";
-    tokenToName[TOKEN_CCONTINUE] = "ccontinue";
     tokenToName[TOKEN_CDO] = "cdo";
     tokenToName[TOKEN_CFOR] = "cfor";
     tokenToName[TOKEN_CIF] = "cif";
     tokenToName[TOKEN_CWHILE] = "cwhile";
     tokenToName[TOKEN_CONST] = "const";
     tokenToName[TOKEN_CONTINUE] = "continue";
-    tokenToName[TOKEN_CRETURN] = "creturn";
     tokenToName[TOKEN_DEFAULT] = "default";
     tokenToName[TOKEN_DO] = "do";
     tokenToName[TOKEN_DELETE] = "delete";
@@ -208,15 +205,12 @@ void ParserInit() {
     tokenNameRemap["TOKEN_BOOL"] = "\'bool\'";
     tokenNameRemap["TOKEN_BREAK"] = "\'break\'";
     tokenNameRemap["TOKEN_CASE"] = "\'case\'";
-    tokenNameRemap["TOKEN_CBREAK"] = "\'cbreak\'";
-    tokenNameRemap["TOKEN_CCONTINUE"] = "\'ccontinue\'";
     tokenNameRemap["TOKEN_CDO"] = "\'cdo\'";
     tokenNameRemap["TOKEN_CFOR"] = "\'cfor\'";
     tokenNameRemap["TOKEN_CIF"] = "\'cif\'";
     tokenNameRemap["TOKEN_CWHILE"] = "\'cwhile\'";
     tokenNameRemap["TOKEN_CONST"] = "\'const\'";
     tokenNameRemap["TOKEN_CONTINUE"] = "\'continue\'";
-    tokenNameRemap["TOKEN_CRETURN"] = "\'creturn\'";
     tokenNameRemap["TOKEN_DEFAULT"] = "\'default\'";
     tokenNameRemap["TOKEN_DO"] = "\'do\'";
     tokenNameRemap["TOKEN_DELETE"] = "\'delete\'";
@@ -351,15 +345,15 @@ __assert { RT; return TOKEN_ASSERT; }
 bool { RT; return TOKEN_BOOL; }
 break { RT; return TOKEN_BREAK; }
 case { RT; return TOKEN_CASE; }
-cbreak { RT; return TOKEN_CBREAK; }
+cbreak { RT; Warning(yylloc, "\"cbreak\" is deprecated. Use \"break\"."); return TOKEN_BREAK; }
-ccontinue { RT; return TOKEN_CCONTINUE; }
+ccontinue { RT; Warning(yylloc, "\"ccontinue\" is deprecated. Use \"continue\"."); return TOKEN_CONTINUE; }
 cdo { RT; return TOKEN_CDO; }
 cfor { RT; return TOKEN_CFOR; }
 cif { RT; return TOKEN_CIF; }
 cwhile { RT; return TOKEN_CWHILE; }
 const { RT; return TOKEN_CONST; }
 continue { RT; return TOKEN_CONTINUE; }
-creturn { RT; return TOKEN_CRETURN; }
+creturn { RT; Warning(yylloc, "\"creturn\" is deprecated. Use \"return\"."); return TOKEN_RETURN; }
 __declspec { RT; return TOKEN_DECLSPEC; }
 default { RT; return TOKEN_DEFAULT; }
 do { RT; return TOKEN_DO; }

parse.yy

@@ -114,8 +114,8 @@ static void lFinalizeEnumeratorSymbols(std::vector<Symbol *> &enums,
                                        const EnumType *enumType);
 
 static const char *lBuiltinTokens[] = {
-    "assert", "bool", "break", "case", "cbreak", "ccontinue", "cdo",
+    "assert", "bool", "break", "case", "cdo",
-    "cfor", "cif", "cwhile", "const", "continue", "creturn", "default",
+    "cfor", "cif", "cwhile", "const", "continue", "default",
     "do", "delete", "double", "else", "enum", "export", "extern", "false",
     "float", "for", "foreach", "foreach_active", "foreach_tiled",
      "foreach_unique", "goto", "if", "in", "inline",
@@ -198,8 +198,8 @@ struct ForeachDimension {
 %token TOKEN_WHILE TOKEN_DO TOKEN_LAUNCH TOKEN_FOREACH TOKEN_FOREACH_TILED 
 %token TOKEN_FOREACH_UNIQUE TOKEN_FOREACH_ACTIVE TOKEN_DOTDOTDOT
 %token TOKEN_FOR TOKEN_GOTO TOKEN_CONTINUE TOKEN_BREAK TOKEN_RETURN
-%token TOKEN_CIF TOKEN_CDO TOKEN_CFOR TOKEN_CWHILE TOKEN_CBREAK
+%token TOKEN_CIF TOKEN_CDO TOKEN_CFOR TOKEN_CWHILE
-%token TOKEN_CCONTINUE TOKEN_CRETURN TOKEN_SYNC TOKEN_PRINT TOKEN_ASSERT
+%token TOKEN_SYNC TOKEN_PRINT TOKEN_ASSERT
 
 %type <expr> primary_expression postfix_expression integer_dotdotdot
 %type <expr> unary_expression cast_expression funcall_expression launch_expression
@@ -1870,21 +1870,13 @@ jump_statement
     : TOKEN_GOTO goto_identifier ';'
       { $$ = new GotoStmt($2, @1, @2); }
     | TOKEN_CONTINUE ';'
-      { $$ = new ContinueStmt(false, @1); }
+      { $$ = new ContinueStmt(@1); }
     | TOKEN_BREAK ';'
-      { $$ = new BreakStmt(false, @1); }
+      { $$ = new BreakStmt(@1); }
     | TOKEN_RETURN ';'
-      { $$ = new ReturnStmt(NULL, false, @1); }
+      { $$ = new ReturnStmt(NULL, @1); }
     | TOKEN_RETURN expression ';'
-      { $$ = new ReturnStmt($2, false, @1); }
+      { $$ = new ReturnStmt($2, @1); }
-    | TOKEN_CCONTINUE ';'
-      { $$ = new ContinueStmt(true, @1); }
-    | TOKEN_CBREAK ';'
-      { $$ = new BreakStmt(true, @1); }
-    | TOKEN_CRETURN ';'
-      { $$ = new ReturnStmt(NULL, true, @1); }
-    | TOKEN_CRETURN expression ';'
-      { $$ = new ReturnStmt($2, true, @1); }
     ;
 
 sync_statement

stmt.cpp

@@ -1178,8 +1178,8 @@ ForStmt::Print(int indent) const {
 ///////////////////////////////////////////////////////////////////////////
 // BreakStmt
 
-BreakStmt::BreakStmt(bool cc, SourcePos p) 
+BreakStmt::BreakStmt(SourcePos p) 
-    : Stmt(p), doCoherenceCheck(cc && !g->opt.disableCoherentControlFlow) {
+    : Stmt(p) {
 }
 
 
@@ -1189,7 +1189,7 @@ BreakStmt::EmitCode(FunctionEmitContext *ctx) const {
         return;
 
     ctx->SetDebugPos(pos);
-    ctx->Break(doCoherenceCheck);
+    ctx->Break(true);
 }
 
 
@@ -1201,14 +1201,13 @@ BreakStmt::TypeCheck() {
 
 int
 BreakStmt::EstimateCost() const {
-    return doCoherenceCheck ? COST_COHERENT_BREAK_CONTINE : 
+    return COST_BREAK_CONTINUE;
-        COST_REGULAR_BREAK_CONTINUE;
 }
 
 
 void
 BreakStmt::Print(int indent) const {
-    printf("%*c%sBreak Stmt", indent, ' ', doCoherenceCheck ? "Coherent " : "");
+    printf("%*cBreak Stmt", indent, ' ');
     pos.Print();
     printf("\n");
 }
@@ -1217,8 +1216,8 @@ BreakStmt::Print(int indent) const {
 ///////////////////////////////////////////////////////////////////////////
 // ContinueStmt
 
-ContinueStmt::ContinueStmt(bool cc, SourcePos p) 
+ContinueStmt::ContinueStmt(SourcePos p) 
-    : Stmt(p), doCoherenceCheck(cc && !g->opt.disableCoherentControlFlow) {
+    : Stmt(p) {
 }
 
 
@@ -1228,7 +1227,7 @@ ContinueStmt::EmitCode(FunctionEmitContext *ctx) const {
         return;
 
     ctx->SetDebugPos(pos);
-    ctx->Continue(doCoherenceCheck);
+    ctx->Continue(true);
 }
 
 
@@ -1240,14 +1239,13 @@ ContinueStmt::TypeCheck() {
 
 int
 ContinueStmt::EstimateCost() const {
-    return doCoherenceCheck ? COST_COHERENT_BREAK_CONTINE : 
+    return COST_BREAK_CONTINUE;
-        COST_REGULAR_BREAK_CONTINUE;
 }
 
 
 void
 ContinueStmt::Print(int indent) const {
-    printf("%*c%sContinue Stmt", indent, ' ', doCoherenceCheck ? "Coherent " : "");
+    printf("%*cContinue Stmt", indent, ' ');
     pos.Print();
     printf("\n");
 }
@@ -2684,9 +2682,8 @@ UnmaskedStmt::EstimateCost() const {
 ///////////////////////////////////////////////////////////////////////////
 // ReturnStmt
 
-ReturnStmt::ReturnStmt(Expr *e, bool cc, SourcePos p) 
+ReturnStmt::ReturnStmt(Expr *e, SourcePos p) 
-    : Stmt(p), expr(e), 
+    : Stmt(p), expr(e) {
-      doCoherenceCheck(cc && !g->opt.disableCoherentControlFlow) {
 }
 
 
@@ -2722,7 +2719,7 @@ ReturnStmt::EmitCode(FunctionEmitContext *ctx) const {
     }
 
     ctx->SetDebugPos(pos);
-    ctx->CurrentLanesReturned(expr, doCoherenceCheck);
+    ctx->CurrentLanesReturned(expr, true);
 }
 
 
@@ -2740,7 +2737,7 @@ ReturnStmt::EstimateCost() const {
 
 void
 ReturnStmt::Print(int indent) const {
-    printf("%*c%sReturn Stmt", indent, ' ', doCoherenceCheck ? "Coherent " : "");
+    printf("%*cReturn Stmt", indent, ' ');
     pos.Print();
     if (expr)
         expr->Print();

stmt.h

@@ -195,45 +195,31 @@ public:
 };
 
 
-/** @brief Statement implementation for a break or 'coherent' break
+/** @brief Statement implementation for a break statement in the
-    statement in the program. */
+    program. */
 class BreakStmt : public Stmt {
 public:
-    BreakStmt(bool doCoherenceCheck, SourcePos pos);
+    BreakStmt(SourcePos pos);
 
     void EmitCode(FunctionEmitContext *ctx) const;
     void Print(int indent) const;
 
     Stmt *TypeCheck();
     int EstimateCost() const;
-
-private:
-    /** This indicates whether the generated code will check to see if no
-        more program instances are currently running after the break, in
-        which case the code can have a jump to the end of the current
-        loop. */
-    const bool doCoherenceCheck;
 };
 
 
-/** @brief Statement implementation for a continue or 'coherent' continue
+/** @brief Statement implementation for a continue statement in the
-    statement in the program. */
+    program. */
 class ContinueStmt : public Stmt {
 public:
-    ContinueStmt(bool doCoherenceCheck, SourcePos pos);
+    ContinueStmt(SourcePos pos);
 
     void EmitCode(FunctionEmitContext *ctx) const;
     void Print(int indent) const;
 
     Stmt *TypeCheck();
     int EstimateCost() const;
-
-private:
-    /** This indicates whether the generated code will check to see if no
-        more program instances are currently running after the continue, in
-        which case the code can have a jump to the end of the current
-        loop. */
-    const bool doCoherenceCheck;
 };
 
 
@@ -314,11 +300,11 @@ public:
 
 
 
-/** @brief Statement implementation for a 'return' or 'coherent' return
+/** @brief Statement implementation for a 'return' statement in the
-    statement in the program. */
+    program. */
 class ReturnStmt : public Stmt {
 public:
-    ReturnStmt(Expr *e, bool cc, SourcePos p);
+    ReturnStmt(Expr *e, SourcePos p);
 
     void EmitCode(FunctionEmitContext *ctx) const;
     void Print(int indent) const;
@@ -327,11 +313,6 @@ public:
     int EstimateCost() const;
 
     Expr *expr;
-    /** This indicates whether the generated code will check to see if no
-        more program instances are currently running after the return, in
-        which case the code can possibly jump to the end of the current
-        function. */
-    const bool doCoherenceCheck;
 };