From 0848c2cc19449bad9a8767e9fa9304c31cd8ccca Mon Sep 17 00:00:00 2001 From: Matt Pharr Date: Tue, 13 Sep 2011 19:48:04 -0700 Subject: [PATCH] Actually make all 'if' statements check for 'all off' mask. Contrary to claims in 0c2048385, that checkin didn't include the changes to not run if/else blocks if none of the program instances wanted to be running them. This checkin fixes that and thus actually fixes issue #74. --- stmt.cpp | 96 ++++++++++++++++++++++++++------------------------------ stmt.h | 6 ++-- 2 files changed, 48 insertions(+), 54 deletions(-) diff --git a/stmt.cpp b/stmt.cpp index c532b75c..a575599f 100644 --- a/stmt.cpp +++ b/stmt.cpp @@ -404,10 +404,10 @@ DeclStmt::Print(int indent) const { IfStmt::IfStmt(Expr *t, Stmt *ts, Stmt *fs, bool checkCoherence, SourcePos p) : Stmt(p), test(t), trueStmts(ts), falseStmts(fs), - doCoherentCheck(checkCoherence && - (test->GetType() != NULL) && - test->GetType()->IsVaryingType() && - !g->opt.disableCoherentControlFlow) { + doAllCheck(checkCoherence && + (test->GetType() != NULL) && + test->GetType()->IsVaryingType() && + !g->opt.disableCoherentControlFlow) { } @@ -439,62 +439,46 @@ IfStmt::EmitCode(FunctionEmitContext *ctx) const { ctx->SetDebugPos(pos); bool isUniform = testType->IsUniformType(); + + llvm::Value *testValue = test->GetValue(ctx); + if (testValue == NULL) + return; + if (isUniform) { ctx->StartUniformIf(ctx->GetMask()); - if (doCoherentCheck) + if (doAllCheck) Warning(test->pos, "Uniform condition supplied to \"cif\" statement."); // 'If' statements with uniform conditions are relatively // straightforward. We evaluate the condition and then jump to // either the 'then' or 'else' clause depending on its value. - llvm::Value *vtest = test->GetValue(ctx); - if (vtest != NULL) { - llvm::BasicBlock *bthen = ctx->CreateBasicBlock("if_then"); - llvm::BasicBlock *belse = ctx->CreateBasicBlock("if_else"); - llvm::BasicBlock *bexit = ctx->CreateBasicBlock("if_exit"); + llvm::BasicBlock *bthen = ctx->CreateBasicBlock("if_then"); + llvm::BasicBlock *belse = ctx->CreateBasicBlock("if_else"); + llvm::BasicBlock *bexit = ctx->CreateBasicBlock("if_exit"); - // Jump to the appropriate basic block based on the value of - // the 'if' test - ctx->BranchInst(bthen, belse, vtest); + // Jump to the appropriate basic block based on the value of + // the 'if' test + ctx->BranchInst(bthen, belse, testValue); - // Emit code for the 'true' case - ctx->SetCurrentBasicBlock(bthen); - lEmitIfStatements(ctx, trueStmts, "true"); - if (ctx->GetCurrentBasicBlock()) - ctx->BranchInst(bexit); + // Emit code for the 'true' case + ctx->SetCurrentBasicBlock(bthen); + lEmitIfStatements(ctx, trueStmts, "true"); + if (ctx->GetCurrentBasicBlock()) + ctx->BranchInst(bexit); - // Emit code for the 'false' case - ctx->SetCurrentBasicBlock(belse); - lEmitIfStatements(ctx, falseStmts, "false"); - if (ctx->GetCurrentBasicBlock()) - ctx->BranchInst(bexit); + // Emit code for the 'false' case + ctx->SetCurrentBasicBlock(belse); + lEmitIfStatements(ctx, falseStmts, "false"); + if (ctx->GetCurrentBasicBlock()) + ctx->BranchInst(bexit); - // Set the active basic block to the newly-created exit block - // so that subsequent emitted code starts there. - ctx->SetCurrentBasicBlock(bexit); - } + // Set the active basic block to the newly-created exit block + // so that subsequent emitted code starts there. + ctx->SetCurrentBasicBlock(bexit); ctx->EndIf(); } - else { - // Code for 'If' statemnts with 'varying' conditions can be - // generated in two ways; one takes some care to see if all of the - // active program instances want to follow only the 'true' or - // 'false' cases, and the other always runs both cases but sets the - // mask appropriately. The first case is handled by the - // IfStmt::emitCoherentTests() call, and the second is handled by - // IfStmt::emitMaskedTrueAndFalse(). - llvm::Value *testValue = test->GetValue(ctx); - if (testValue) { - if (doCoherentCheck) - emitCoherentTests(ctx, testValue); - else { - llvm::Value *oldMask = ctx->GetMask(); - ctx->StartVaryingIf(oldMask); - emitMaskedTrueAndFalse(ctx, oldMask, testValue); - ctx->EndIf(); - } - } - } + else + emitVaryingIf(ctx, testValue); } @@ -540,7 +524,7 @@ Stmt *IfStmt::TypeCheck() { void IfStmt::Print(int indent) const { - printf("%*cIf Stmt %s", indent, ' ', doCoherentCheck ? "DO COHERENT CHECK" : ""); + printf("%*cIf Stmt %s", indent, ' ', doAllCheck ? "DO ALL CHECK" : ""); pos.Print(); printf("\n%*cTest: ", indent+4, ' '); test->Print(); @@ -581,7 +565,7 @@ IfStmt::emitMaskedTrueAndFalse(FunctionEmitContext *ctx, llvm::Value *oldMask, tries to be smart about jumping over code that doesn't need to be run. */ void -IfStmt::emitCoherentTests(FunctionEmitContext *ctx, llvm::Value *ltest) const { +IfStmt::emitVaryingIf(FunctionEmitContext *ctx, llvm::Value *ltest) const { llvm::Value *oldMask = ctx->GetMask(); if (oldMask == LLVMMaskAllOn) { // We can tell that the mask is on statically at compile time; just @@ -590,7 +574,7 @@ IfStmt::emitCoherentTests(FunctionEmitContext *ctx, llvm::Value *ltest) const { emitMaskAllOn(ctx, ltest, bDone); ctx->SetCurrentBasicBlock(bDone); } - else { + else if (doAllCheck) { // We can't tell if the mask going into the if is all on at the // compile time. Emit code to check for this and then either run // the code for the 'all on' or the 'mixed' case depending on the @@ -618,6 +602,16 @@ IfStmt::emitCoherentTests(FunctionEmitContext *ctx, llvm::Value *ltest) const { ctx->SetCurrentBasicBlock(bMixedOn); emitMaskMixed(ctx, oldMask, ltest, bDone); + // When done, set the current basic block to the block that the two + // paths above jump to when they're done. + ctx->SetCurrentBasicBlock(bDone); + } + else { + llvm::BasicBlock *bDone = ctx->CreateBasicBlock("cif_done"); + + // Emit emit code for the mixed mask case + emitMaskMixed(ctx, oldMask, ltest, bDone); + // When done, set the current basic block to the block that the two // paths above jump to when they're done. ctx->SetCurrentBasicBlock(bDone); @@ -701,7 +695,7 @@ IfStmt::emitMaskMixed(FunctionEmitContext *ctx, llvm::Value *oldMask, // value of the test is on. (i.e. (test&mask) == mask). In this case, // we only need to run the 'true' case code, since the lanes where the // test was false aren't supposed to be running here anyway. - llvm::Value *testAllEqual = lTestMatchesMask(ctx, ltest, oldMask); + llvm::Value *testAllEqual = lTestMatchesMask(ctx, ltest, oldMask); llvm::BasicBlock *bTestAll = ctx->CreateBasicBlock("cif_mixed_test_all"); llvm::BasicBlock *bTestAnyCheck = ctx->CreateBasicBlock("cif_mixed_test_any_check"); ctx->BranchInst(bTestAll, bTestAnyCheck, testAllEqual); diff --git a/stmt.h b/stmt.h index 9d469b85..0e60b67c 100644 --- a/stmt.h +++ b/stmt.h @@ -103,7 +103,7 @@ private: class IfStmt : public Stmt { public: IfStmt(Expr *testExpr, Stmt *trueStmts, Stmt *falseStmts, - bool doCoherentCheck, SourcePos pos); + bool doAllCheck, SourcePos pos); void EmitCode(FunctionEmitContext *ctx) const; void Print(int indent) const; @@ -125,11 +125,11 @@ private: source and thus, if the emitted code should check to see if all active program instances want to follow just one of the 'true' or 'false' blocks. */ - const bool doCoherentCheck; + const bool doAllCheck; void emitMaskedTrueAndFalse(FunctionEmitContext *ctx, llvm::Value *oldMask, llvm::Value *test) const; - void emitCoherentTests(FunctionEmitContext *ctx, llvm::Value *test) const; + void emitVaryingIf(FunctionEmitContext *ctx, llvm::Value *test) const; void emitMaskAllOn(FunctionEmitContext *ctx, llvm::Value *test, llvm::BasicBlock *bDone) const; void emitMaskMixed(FunctionEmitContext *ctx, llvm::Value *oldMask,