Skip to content

S
swiftshader
Commit 6d12331a by Nicolas Capens Committed by Nicolas Capens
Options

Fix break statement.

The break 'depth' was used to indicate the number of if/else execution enable mask's stack levels we need to discard when jumping from a break statement to its enclosing switch or loop. However, each switch and loop resets this depth at the end, which isn't correct for nested loops and/or switches (note that switches contain if/else statements, and loops use the same 'enable' masks as if/else). This can be fixed either by using a stack to keep track of the break depths of nested switch/loop statements, or by simply not jumping directly from the break statement to the end of it's enclosing switch or loop. The latter fix was chosen for this change, which assumes that that it's uncommon for all vector lanes to become disabled at the break statement and skip many instructions. An important exception to this is breaking out of an infinite or long-running loop, but this is handled by checking the break enable mask as part of the loop condition. Change-Id: I57d2e03941e855faefd997442931ff8619eca73f Reviewed-on: https://swiftshader-review.googlesource.com/15968Tested-by: 's avatarNicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarAlexis Hétu <sugoi@google.com> Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com>
parent d6bcc111
Showing with 12 additions and 75 deletions
src/OpenGL/compiler/OutputASM.cpp
...@@ -1772,10 +1772,16 @@ namespace glsl ...@@ -1772,10 +1772,16 @@ namespace glsl
emit(sw::Shader::OPCODE_IF, 0, &result); emit(sw::Shader::OPCODE_IF, 0, &result);
nbCases++; nbCases++;
// Emit the code for this case and all subsequent cases until we hit a break statement.
// TODO: This can repeat a lot of code for switches with many fall-through cases.
for(++caseIt; caseIt != sequence.end(); ++caseIt) for(++caseIt; caseIt != sequence.end(); ++caseIt)
{ {
(*caseIt)->traverse(this); (*caseIt)->traverse(this);
if((*caseIt)->getAsBranchNode()) // Kill, Break, Continue or Return
// Stop if we encounter an unconditional branch (break, continue, return, or kill).
// TODO: This doesn't work if the statement is at a deeper scope level (e.g. {break;}).
// Note that this eliminates useless operations but shouldn't affect correctness.
if((*caseIt)->getAsBranchNode())
{ {
break; break;
} }
......
src/Shader/PixelProgram.cpp
...@@ -1251,25 +1251,7 @@ namespace sw ...@@ -1251,25 +1251,7 @@ namespace sw
void PixelProgram::BREAK() void PixelProgram::BREAK()
{ {
BasicBlock *deadBlock = Nucleus::createBasicBlock(); enableBreak = enableBreak & ~enableStack[enableIndex];
BasicBlock *endBlock = loopRepEndBlock[loopRepDepth - 1];
if(breakDepth == 0)
{
enableIndex = enableIndex - breakDepth;
Nucleus::createBr(endBlock);
}
else
{
enableBreak = enableBreak & ~enableStack[enableIndex];
Bool allBreak = SignMask(enableBreak) == 0x0;
enableIndex = enableIndex - breakDepth;
branch(allBreak, endBlock, deadBlock);
}
Nucleus::setInsertBlock(deadBlock);
enableIndex = enableIndex + breakDepth;
} }
void PixelProgram::BREAKC(Vector4f &src0, Vector4f &src1, Control control) void PixelProgram::BREAKC(Vector4f &src0, Vector4f &src1, Control control)
...@@ -1307,17 +1289,7 @@ namespace sw ...@@ -1307,17 +1289,7 @@ namespace sw
{ {
condition &= enableStack[enableIndex]; condition &= enableStack[enableIndex];
BasicBlock *continueBlock = Nucleus::createBasicBlock();
BasicBlock *endBlock = loopRepEndBlock[loopRepDepth - 1];
enableBreak = enableBreak & ~condition; enableBreak = enableBreak & ~condition;
Bool allBreak = SignMask(enableBreak) == 0x0;
enableIndex = enableIndex - breakDepth;
branch(allBreak, endBlock, continueBlock);
Nucleus::setInsertBlock(continueBlock);
enableIndex = enableIndex + breakDepth;
} }
void PixelProgram::CONTINUE() void PixelProgram::CONTINUE()
...@@ -1461,7 +1433,6 @@ namespace sw ...@@ -1461,7 +1433,6 @@ namespace sw
if(isConditionalIf[ifDepth]) if(isConditionalIf[ifDepth])
{ {
breakDepth--;
enableIndex--; enableIndex--;
} }
} }
...@@ -1608,7 +1579,6 @@ namespace sw ...@@ -1608,7 +1579,6 @@ namespace sw
ifFalseBlock[ifDepth] = falseBlock; ifFalseBlock[ifDepth] = falseBlock;
ifDepth++; ifDepth++;
breakDepth++;
} }
void PixelProgram::LABEL(int labelIndex) void PixelProgram::LABEL(int labelIndex)
...@@ -1652,7 +1622,6 @@ namespace sw ...@@ -1652,7 +1622,6 @@ namespace sw
iteration[loopDepth] = iteration[loopDepth] - 1; // FIXME: -- iteration[loopDepth] = iteration[loopDepth] - 1; // FIXME: --
loopRepDepth++; loopRepDepth++;
breakDepth = 0;
} }
void PixelProgram::REP(const Src &integerRegister) void PixelProgram::REP(const Src &integerRegister)
...@@ -1679,7 +1648,6 @@ namespace sw ...@@ -1679,7 +1648,6 @@ namespace sw
iteration[loopDepth] = iteration[loopDepth] - 1; // FIXME: -- iteration[loopDepth] = iteration[loopDepth] - 1; // FIXME: --
loopRepDepth++; loopRepDepth++;
breakDepth = 0;
} }
void PixelProgram::WHILE(const Src &temporaryRegister) void PixelProgram::WHILE(const Src &temporaryRegister)
...@@ -1705,6 +1673,7 @@ namespace sw ...@@ -1705,6 +1673,7 @@ namespace sw
Int4 condition = As<Int4>(src.x); Int4 condition = As<Int4>(src.x);
condition &= enableStack[enableIndex - 1]; condition &= enableStack[enableIndex - 1];
if(shader->containsLeaveInstruction()) condition &= enableLeave; if(shader->containsLeaveInstruction()) condition &= enableLeave;
if(shader->containsBreakInstruction()) condition &= enableBreak;
enableStack[enableIndex] = condition; enableStack[enableIndex] = condition;
Bool notAllFalse = SignMask(condition) != 0; Bool notAllFalse = SignMask(condition) != 0;
...@@ -1716,7 +1685,6 @@ namespace sw ...@@ -1716,7 +1685,6 @@ namespace sw
Nucleus::setInsertBlock(loopBlock); Nucleus::setInsertBlock(loopBlock);
loopRepDepth++; loopRepDepth++;
breakDepth = 0;
} }
void PixelProgram::SWITCH() void PixelProgram::SWITCH()
...@@ -1736,7 +1704,6 @@ namespace sw ...@@ -1736,7 +1704,6 @@ namespace sw
Nucleus::setInsertBlock(currentBlock); Nucleus::setInsertBlock(currentBlock);
loopRepDepth++; loopRepDepth++;
breakDepth = 0;
} }
void PixelProgram::RET() void PixelProgram::RET()
......
src/Shader/PixelProgram.hpp
...@@ -25,7 +25,7 @@ namespace sw ...@@ -25,7 +25,7 @@ namespace sw
public: public:
PixelProgram(const PixelProcessor::State &state, const PixelShader *shader) : PixelProgram(const PixelProcessor::State &state, const PixelShader *shader) :
PixelRoutine(state, shader), r(shader && shader->dynamicallyIndexedTemporaries), PixelRoutine(state, shader), r(shader && shader->dynamicallyIndexedTemporaries),
loopDepth(-1), ifDepth(0), loopRepDepth(0), breakDepth(0), currentLabel(-1), whileTest(false) loopDepth(-1), ifDepth(0), loopRepDepth(0), currentLabel(-1), whileTest(false)
{ {
for(int i = 0; i < 2048; ++i) for(int i = 0; i < 2048; ++i)
{ {
...@@ -153,7 +153,6 @@ namespace sw ...@@ -153,7 +153,6 @@ namespace sw
int ifDepth; int ifDepth;
int loopRepDepth; int loopRepDepth;
int breakDepth;
int currentLabel; int currentLabel;
bool whileTest; bool whileTest;
......
src/Shader/VertexProgram.cpp
...@@ -28,7 +28,6 @@ namespace sw ...@@ -28,7 +28,6 @@ namespace sw
{ {
ifDepth = 0; ifDepth = 0;
loopRepDepth = 0; loopRepDepth = 0;
breakDepth = 0;
currentLabel = -1; currentLabel = -1;
whileTest = false; whileTest = false;
...@@ -1011,25 +1010,7 @@ namespace sw ...@@ -1011,25 +1010,7 @@ namespace sw
void VertexProgram::BREAK() void VertexProgram::BREAK()
{ {
BasicBlock *deadBlock = Nucleus::createBasicBlock(); enableBreak = enableBreak & ~enableStack[enableIndex];
BasicBlock *endBlock = loopRepEndBlock[loopRepDepth - 1];
if(breakDepth == 0)
{
enableIndex = enableIndex - breakDepth;
Nucleus::createBr(endBlock);
}
else
{
enableBreak = enableBreak & ~enableStack[enableIndex];
Bool allBreak = SignMask(enableBreak) == 0x0;
enableIndex = enableIndex - breakDepth;
branch(allBreak, endBlock, deadBlock);
}
Nucleus::setInsertBlock(deadBlock);
enableIndex = enableIndex + breakDepth;
} }
void VertexProgram::BREAKC(Vector4f &src0, Vector4f &src1, Control control) void VertexProgram::BREAKC(Vector4f &src0, Vector4f &src1, Control control)
...@@ -1067,17 +1048,7 @@ namespace sw ...@@ -1067,17 +1048,7 @@ namespace sw
{ {
condition &= enableStack[enableIndex]; condition &= enableStack[enableIndex];
BasicBlock *continueBlock = Nucleus::createBasicBlock();
BasicBlock *endBlock = loopRepEndBlock[loopRepDepth - 1];
enableBreak = enableBreak & ~condition; enableBreak = enableBreak & ~condition;
Bool allBreak = SignMask(enableBreak) == 0x0;
enableIndex = enableIndex - breakDepth;
branch(allBreak, endBlock, continueBlock);
Nucleus::setInsertBlock(continueBlock);
enableIndex = enableIndex + breakDepth;
} }
void VertexProgram::CONTINUE() void VertexProgram::CONTINUE()
...@@ -1221,7 +1192,6 @@ namespace sw ...@@ -1221,7 +1192,6 @@ namespace sw
if(isConditionalIf[ifDepth]) if(isConditionalIf[ifDepth])
{ {
breakDepth--;
enableIndex--; enableIndex--;
} }
} }
...@@ -1368,7 +1338,6 @@ namespace sw ...@@ -1368,7 +1338,6 @@ namespace sw
ifFalseBlock[ifDepth] = falseBlock; ifFalseBlock[ifDepth] = falseBlock;
ifDepth++; ifDepth++;
breakDepth++;
} }
void VertexProgram::LABEL(int labelIndex) void VertexProgram::LABEL(int labelIndex)
...@@ -1413,7 +1382,6 @@ namespace sw ...@@ -1413,7 +1382,6 @@ namespace sw
iteration[loopDepth] = iteration[loopDepth] - 1; // FIXME: -- iteration[loopDepth] = iteration[loopDepth] - 1; // FIXME: --
loopRepDepth++; loopRepDepth++;
breakDepth = 0;
} }
void VertexProgram::REP(const Src &integerRegister) void VertexProgram::REP(const Src &integerRegister)
...@@ -1440,7 +1408,6 @@ namespace sw ...@@ -1440,7 +1408,6 @@ namespace sw
iteration[loopDepth] = iteration[loopDepth] - 1; // FIXME: -- iteration[loopDepth] = iteration[loopDepth] - 1; // FIXME: --
loopRepDepth++; loopRepDepth++;
breakDepth = 0;
} }
void VertexProgram::WHILE(const Src &temporaryRegister) void VertexProgram::WHILE(const Src &temporaryRegister)
...@@ -1466,6 +1433,7 @@ namespace sw ...@@ -1466,6 +1433,7 @@ namespace sw
Int4 condition = As<Int4>(src.x); Int4 condition = As<Int4>(src.x);
condition &= enableStack[enableIndex - 1]; condition &= enableStack[enableIndex - 1];
if(shader->containsLeaveInstruction()) condition &= enableLeave; if(shader->containsLeaveInstruction()) condition &= enableLeave;
if(shader->containsBreakInstruction()) condition &= enableBreak;
enableStack[enableIndex] = condition; enableStack[enableIndex] = condition;
Bool notAllFalse = SignMask(condition) != 0; Bool notAllFalse = SignMask(condition) != 0;
...@@ -1477,7 +1445,6 @@ namespace sw ...@@ -1477,7 +1445,6 @@ namespace sw
Nucleus::setInsertBlock(loopBlock); Nucleus::setInsertBlock(loopBlock);
loopRepDepth++; loopRepDepth++;
breakDepth = 0;
} }
void VertexProgram::SWITCH() void VertexProgram::SWITCH()
...@@ -1497,7 +1464,6 @@ namespace sw ...@@ -1497,7 +1464,6 @@ namespace sw
Nucleus::setInsertBlock(currentBlock); Nucleus::setInsertBlock(currentBlock);
loopRepDepth++; loopRepDepth++;
breakDepth = 0;
} }
void VertexProgram::RET() void VertexProgram::RET()
......
src/Shader/VertexProgram.hpp
...@@ -122,7 +122,6 @@ namespace sw ...@@ -122,7 +122,6 @@ namespace sw
int ifDepth; int ifDepth;
int loopRepDepth; int loopRepDepth;
int breakDepth;
int currentLabel; int currentLabel;
bool whileTest; bool whileTest;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment