Short-circuit evaluation of ? : operator for varying tests.

? : now short-circuits evaluation of the expressions following
the boolean test for varying test types.  (It already did this
for uniform tests).

Issue #169.

docs/ispc.rst

@@ -1151,6 +1151,7 @@ in C:
 * Structs and arrays
 * Support for recursive function calls
 * Support for separate compilation of source files
+* "Short-circuit" evaluation of ``||``, ``&&`` and ``? :`` operators
 * The preprocessor
 
 ``ispc`` adds a number of features from C++ and C99 to this base:
@@ -1968,11 +1969,12 @@ operator also work as expected.
     (*fp).a = 0;
     fp->b = 1;
   
-As in C and C++, evaluation of the ``||`` and ``&&`` logical operators is
+As in C and C++, evaluation of the ``||`` and ``&&`` logical operators as
-"short-circuited"; the right hand side won't be evaluated if the value from
+well as the selection operator ``? :`` is "short-circuited"; the right hand
-the left-hand side determines the logical operator's value.  For example,
+side won't be evaluated if the value from the left-hand side determines the
-in the following code, ``array[index]`` won't be evaluated for values of
+logical operator's value.  For example, in the following code,
-``index`` that are greater than or equal to ``NUM_ITEMS``.
+``array[index]`` won't be evaluated for values of ``index`` that are
+greater than or equal to ``NUM_ITEMS``.
 
 ::
 

expr.cpp

@@ -2686,6 +2686,34 @@ lEmitVaryingSelect(FunctionEmitContext *ctx, llvm::Value *test,
 }
 
 
+static void
+lEmitSelectExprCode(FunctionEmitContext *ctx, llvm::Value *testVal, 
+                    llvm::Value *oldMask, llvm::Value *fullMask,
+                    Expr *expr, llvm::Value *exprPtr) {
+    llvm::BasicBlock *bbEval = ctx->CreateBasicBlock("select_eval_expr");
+    llvm::BasicBlock *bbDone = ctx->CreateBasicBlock("select_done");
+
+    // Check to see if the test was true for any of the currently executing
+    // program instances.
+    llvm::Value *testAndFullMask = 
+        ctx->BinaryOperator(llvm::Instruction::And, testVal, fullMask, 
+                            "test&mask");
+    llvm::Value *anyOn = ctx->Any(testAndFullMask);
+    ctx->BranchInst(bbEval, bbDone, anyOn);
+
+    ctx->SetCurrentBasicBlock(bbEval);
+    llvm::Value *testAndMask = 
+        ctx->BinaryOperator(llvm::Instruction::And, testVal, oldMask, 
+                            "test&mask");
+    ctx->SetInternalMask(testAndMask);
+    llvm::Value *exprVal = expr->GetValue(ctx);
+    ctx->StoreInst(exprVal, exprPtr);
+    ctx->BranchInst(bbDone);
+
+    ctx->SetCurrentBasicBlock(bbDone);
+}
+
+
 llvm::Value *
 SelectExpr::GetValue(FunctionEmitContext *ctx) const {
     if (!expr1 || !expr2 || !test)
@@ -2733,18 +2761,58 @@ SelectExpr::GetValue(FunctionEmitContext *ctx) const {
         return ret;
     }
     else if (dynamic_cast<const VectorType *>(testType) == NULL) {
-        // if the test is a varying bool type, then evaluate both of the
+        // the test is a varying bool type
-        // value expressions with the mask set appropriately and then do an
-        // element-wise select to get the result
         llvm::Value *testVal = test->GetValue(ctx);
         Assert(testVal->getType() == LLVMTypes::MaskType);
         llvm::Value *oldMask = ctx->GetInternalMask();
+        llvm::Value *fullMask = ctx->GetFullMask();
+
+        // We don't want to incur the overhead for short-circuit evaluation
+        // for expressions that are both computationally simple and safe to
+        // run with an "all off" mask.
+        bool shortCircuit1 =
+            (::EstimateCost(expr1) > PREDICATE_SAFE_IF_STATEMENT_COST ||
+             SafeToRunWithMaskAllOff(expr1) == false);
+        bool shortCircuit2 =
+            (::EstimateCost(expr2) > PREDICATE_SAFE_IF_STATEMENT_COST ||
+             SafeToRunWithMaskAllOff(expr2) == false);
+
+        Debug(expr1->pos, "%sshort circuiting evaluation for select expr",
+              shortCircuit1 ? "" : "Not ");
+        Debug(expr2->pos, "%sshort circuiting evaluation for select expr",
+              shortCircuit2 ? "" : "Not ");
+
+        // Temporary storage to store the values computed for each
+        // expression, if any.  (These stay as uninitialized memory if we
+        // short circuit around the corresponding expression.)
+        LLVM_TYPE_CONST llvm::Type *exprType = 
+            expr1->GetType()->LLVMType(g->ctx);
+        llvm::Value *expr1Ptr = ctx->AllocaInst(exprType);
+        llvm::Value *expr2Ptr = ctx->AllocaInst(exprType);
+
+        if (shortCircuit1)
+            lEmitSelectExprCode(ctx, testVal, oldMask, fullMask, expr1, 
+                                expr1Ptr);
+        else {
             ctx->SetInternalMaskAnd(oldMask, testVal);
             llvm::Value *expr1Val = expr1->GetValue(ctx);
+            ctx->StoreInst(expr1Val, expr1Ptr);
+        }
+
+        if (shortCircuit2) {
+            llvm::Value *notTest = ctx->NotOperator(testVal);
+            lEmitSelectExprCode(ctx, notTest, oldMask, fullMask, expr2, 
+                                expr2Ptr);
+        }
+        else {
             ctx->SetInternalMaskAndNot(oldMask, testVal);
             llvm::Value *expr2Val = expr2->GetValue(ctx);
-        ctx->SetInternalMask(oldMask);
+            ctx->StoreInst(expr2Val, expr2Ptr);
+        }
 
+        ctx->SetInternalMask(oldMask);
+        llvm::Value *expr1Val = ctx->LoadInst(expr1Ptr);
+        llvm::Value *expr2Val = ctx->LoadInst(expr2Ptr);
         return lEmitVaryingSelect(ctx, testVal, expr1Val, expr2Val, type);
     }
     else {

tests/foreach-double-1.ispc

@@ -0,0 +1,30 @@
+
+export uniform int width() { return programCount; }
+
+uniform double one = 1;
+
+void copy(uniform double dst[], uniform double src[], uniform int count) {
+    foreach (i = 0 ... count)
+        dst[i] = one * src[i];
+}
+
+export void f_f(uniform float RET[], uniform float aFOO[]) {
+    uniform int count = 200 + aFOO[1];
+    uniform double * uniform src = uniform new uniform double[count];
+    for (uniform int i = 0; i < count; ++i)
+        src[i] = i;
+
+    uniform double * uniform dst = uniform new uniform double[count];
+    copy(dst, src, count);
+
+    uniform int errors = 0;
+    for (uniform int i = 0; i < count; ++i)
+        if (dst[i] != src[i])
+            ++errors;
+
+    RET[programIndex] = errors; 
+}
+
+export void result(uniform float RET[]) {
+    RET[programIndex] = 0;
+}

tests/short-circuit-select-1.ispc

@@ -0,0 +1,21 @@
+
+export uniform int width() { return programCount; }
+
+uniform int * uniform ptr;
+
+bool crashEven() {
+    return (programIndex & 1) ? true : (*ptr > 0);
+}
+
+export void f_fu(uniform float RET[], uniform float aFOO[], uniform float b) {
+    float a = aFOO[programIndex]; 
+    float a0 = aFOO[0], a1 = aFOO[1];
+    if (((programIndex & 1) == 0) || crashEven())
+        RET[programIndex] = 1;
+    else
+        RET[programIndex] = 0;
+}
+
+export void result(uniform float RET[]) {
+    RET[programIndex] = 1;
+}

tests/short-circuit-select-2.ispc

@@ -0,0 +1,21 @@
+
+export uniform int width() { return programCount; }
+
+uniform int * uniform ptr;
+
+bool crashEven() {
+    return (programIndex & 1) ? true : (*ptr > 0);
+}
+
+export void f_fu(uniform float RET[], uniform float aFOO[], uniform float b) {
+    float a = aFOO[programIndex]; 
+    float a0 = aFOO[0], a1 = aFOO[1];
+    if (((programIndex & 1) == 1) && crashEven())
+        RET[programIndex] = 1;
+    else
+        RET[programIndex] = 2;
+}
+
+export void result(uniform float RET[]) {
+    RET[programIndex] = (programIndex & 1) ? 1 : 2;
+}

tests/short-circuit-select-3.ispc

@@ -0,0 +1,20 @@
+
+export uniform int width() { return programCount; }
+
+float crashEven(uniform float a[]) {
+    int offset = 0;
+    return (programIndex & 1) ? a[offset] : a[offset+1000000];
+}
+
+export void f_fu(uniform float RET[], uniform float aFOO[], uniform float b) {
+    float a = aFOO[programIndex]; 
+    float a0 = aFOO[0], a1 = aFOO[1];
+    if (((programIndex & 1) == 1) && (crashEven(aFOO) == 1))
+        RET[programIndex] = 1;
+    else
+        RET[programIndex] = 2;
+}
+
+export void result(uniform float RET[]) {
+    RET[programIndex] = (programIndex & 1) ? 1 : 2;
+}