From 89cb809922f5e8598c7f88d09871e7e97fad8dbe Mon Sep 17 00:00:00 2001 From: Matt Pharr Date: Wed, 1 Feb 2012 11:03:58 -0800 Subject: [PATCH] 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 | 12 +++-- expr.cpp | 84 ++++++++++++++++++++++++++++--- tests/foreach-double-1.ispc | 30 +++++++++++ tests/short-circuit-select-1.ispc | 21 ++++++++ tests/short-circuit-select-2.ispc | 21 ++++++++ tests/short-circuit-select-3.ispc | 20 ++++++++ 6 files changed, 175 insertions(+), 13 deletions(-) create mode 100644 tests/foreach-double-1.ispc create mode 100644 tests/short-circuit-select-1.ispc create mode 100644 tests/short-circuit-select-2.ispc create mode 100644 tests/short-circuit-select-3.ispc diff --git a/docs/ispc.rst b/docs/ispc.rst index ee838ee0..345d6119 100644 --- a/docs/ispc.rst +++ b/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 -"short-circuited"; the right hand side won't be evaluated if the value from -the left-hand side determines the logical operator's value. For example, -in the following code, ``array[index]`` won't be evaluated for values of -``index`` that are greater than or equal to ``NUM_ITEMS``. +As in C and C++, evaluation of the ``||`` and ``&&`` logical operators as +well as the selection operator ``? :`` is "short-circuited"; the right hand +side won't be evaluated if the value from the left-hand side determines the +logical operator's value. For example, in the following code, +``array[index]`` won't be evaluated for values of ``index`` that are +greater than or equal to ``NUM_ITEMS``. :: diff --git a/expr.cpp b/expr.cpp index 14415beb..b1194603 100644 --- a/expr.cpp +++ b/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(testType) == NULL) { - // if the test is a varying bool type, then evaluate both of the - // value expressions with the mask set appropriately and then do an - // element-wise select to get the result + // the test is a varying bool type llvm::Value *testVal = test->GetValue(ctx); Assert(testVal->getType() == LLVMTypes::MaskType); llvm::Value *oldMask = ctx->GetInternalMask(); - ctx->SetInternalMaskAnd(oldMask, testVal); - llvm::Value *expr1Val = expr1->GetValue(ctx); - ctx->SetInternalMaskAndNot(oldMask, testVal); - llvm::Value *expr2Val = expr2->GetValue(ctx); - ctx->SetInternalMask(oldMask); + 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->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 { diff --git a/tests/foreach-double-1.ispc b/tests/foreach-double-1.ispc new file mode 100644 index 00000000..16d7cfd0 --- /dev/null +++ b/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; +} diff --git a/tests/short-circuit-select-1.ispc b/tests/short-circuit-select-1.ispc new file mode 100644 index 00000000..2189710a --- /dev/null +++ b/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; +} diff --git a/tests/short-circuit-select-2.ispc b/tests/short-circuit-select-2.ispc new file mode 100644 index 00000000..c152af71 --- /dev/null +++ b/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; +} diff --git a/tests/short-circuit-select-3.ispc b/tests/short-circuit-select-3.ispc new file mode 100644 index 00000000..4b503fc8 --- /dev/null +++ b/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; +}