Skip to content

S
swiftshader
Commit 493fc54a by Nicolas Capens Committed by Nicolas Capens
Options

Fix loop unrolling.

Loops were no longer getting unrolled because we analyzed the entire loop, not just the body, for changes to the index variable. Also, the logic was inverted. Also, be conservative about loops with return statements not being unrollable. Bug chromium:845103 Change-Id: I5957e17f7b985ae90b10053216b6945f3f64338e Reviewed-on: https://swiftshader-review.googlesource.com/19008Tested-by: 's avatarNicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarAlexis Hétu <sugoi@google.com>
parent 937e6a5a
Showing with 209 additions and 40 deletions
src/OpenGL/compiler/OutputASM.cpp
......@@ -1872,6 +1872,8 @@ namespace glsl
if(unroll)
{
mContext.info(node->getLine(), "loop unrolled", "for");
for(unsigned int i = 0; i < loop.iterations; i++)
{
// condition->traverse(this); // Condition could contain statements, but not in an unrollable loop
......@@ -3886,30 +3888,19 @@ namespace glsl
}
}
bool LoopUnrollable::traverse(TIntermNode *node, int indexId)
bool LoopUnrollable::traverse(TIntermLoop *loop, int indexId)
{
loopUnrollable = true;
loopDepth = 0;
loopIndexId = indexId;
TIntermNode *body = loop->getBody();
node->traverse(this);
return loopUnrollable;
}
bool LoopUnrollable::visitLoop(Visit visit, TIntermLoop *loop)
{
if(visit == PreVisit)
{
loopDepth++;
}
else if(visit == PostVisit)
if(body)
{
loopDepth++;
body->traverse(this);
}
return true;
return loopUnrollable;
}
void LoopUnrollable::visitSymbol(TIntermSymbol *node)
......@@ -3933,7 +3924,7 @@ namespace glsl
// Check that the loop index is not statically assigned to.
TIntermSymbol *symbol = node->getLeft()->getAsSymbolNode();
loopUnrollable = node->modifiesState() && symbol && (symbol->getId() == loopIndexId);
loopUnrollable = !(node->modifiesState() && symbol && (symbol->getId() == loopIndexId));
return loopUnrollable;
}
......@@ -3947,7 +3938,7 @@ namespace glsl
// Check that the loop index is not statically assigned to.
TIntermSymbol *symbol = node->getOperand()->getAsSymbolNode();
loopUnrollable = node->modifiesState() && symbol && (symbol->getId() == loopIndexId);
loopUnrollable = !(node->modifiesState() && symbol && (symbol->getId() == loopIndexId));
return loopUnrollable;
}
......@@ -3959,16 +3950,10 @@ namespace glsl
return false;
}
if(!loopDepth)
{
return true;
}
switch(node->getFlowOp())
{
case EOpKill:
case EOpReturn:
break;
case EOpBreak:
case EOpContinue:
loopUnrollable = false;
......
src/OpenGL/compiler/OutputASM.h
......@@ -382,19 +382,17 @@ namespace glsl
class LoopUnrollable : public TIntermTraverser
{
public:
bool traverse(TIntermNode *node, int loopIndexId);
bool traverse(TIntermLoop *loop, int loopIndexId);
private:
void visitSymbol(TIntermSymbol *node) override;
bool visitBinary(Visit visit, TIntermBinary *node) override;
bool visitUnary(Visit visit, TIntermUnary *node) override;
bool visitBranch(Visit visit, TIntermBranch *node) override;
bool visitLoop(Visit visit, TIntermLoop *loop) override;
bool visitAggregate(Visit visit, TIntermAggregate *node) override;
bool loopUnrollable;
int loopDepth;
int loopIndexId;
};
}
......
tests/unittests/unittests.cpp
......@@ -24,6 +24,8 @@
#include <Windows.h>
#endif
#include <string.h>
#define EXPECT_GLENUM_EQ(expected, actual) EXPECT_EQ(static_cast<GLenum>(expected), static_cast<GLenum>(actual))
class SwiftShaderTest : public testing::Test
......@@ -207,8 +209,8 @@ protected:
struct ProgramHandles
{
GLuint program;
GLuint vsShader;
GLuint fsShader;
GLuint vertexShader;
GLuint fragmentShader;
};
ProgramHandles createProgram(const std::string& vs, const std::string& fs)
......@@ -217,20 +219,20 @@ protected:
ph.program = glCreateProgram();
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
ph.vsShader = glCreateShader(GL_VERTEX_SHADER);
ph.vertexShader = glCreateShader(GL_VERTEX_SHADER);
const char* vsSource[1] = { vs.c_str() };
glShaderSource(ph.vsShader, 1, vsSource, nullptr);
glCompileShader(ph.vsShader);
glShaderSource(ph.vertexShader, 1, vsSource, nullptr);
glCompileShader(ph.vertexShader);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
ph.fsShader = glCreateShader(GL_FRAGMENT_SHADER);
ph.fragmentShader = glCreateShader(GL_FRAGMENT_SHADER);
const char* fsSource[1] = { fs.c_str() };
glShaderSource(ph.fsShader, 1, fsSource, nullptr);
glCompileShader(ph.fsShader);
glShaderSource(ph.fragmentShader, 1, fsSource, nullptr);
glCompileShader(ph.fragmentShader);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
glAttachShader(ph.program, ph.vsShader);
glAttachShader(ph.program, ph.fsShader);
glAttachShader(ph.program, ph.vertexShader);
glAttachShader(ph.program, ph.fragmentShader);
glLinkProgram(ph.program);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
......@@ -243,12 +245,12 @@ protected:
void deleteProgram(const ProgramHandles& ph)
{
glDeleteShader(ph.fsShader);
glDeleteShader(ph.vsShader);
glDeleteShader(ph.fragmentShader);
glDeleteShader(ph.vertexShader);
glDeleteProgram(ph.program);
}
void drawQuad(GLuint program, const char* textureName)
void drawQuad(GLuint program, const char* textureName = nullptr)
{
GLint prevProgram = 0;
glGetIntegerv(GL_CURRENT_PROGRAM, &prevProgram);
......@@ -315,6 +317,190 @@ TEST_F(SwiftShaderTest, Initalization)
Uninitialize();
}
// Test unrolling of a loop
TEST_F(SwiftShaderTest, UnrollLoop)
{
Initialize(3, false);
unsigned char green[4] = { 0, 255, 0, 255 };
const std::string vs =
"#version 300 es\n"
"in vec4 position;\n"
"out vec4 color;\n"
"void main()\n"
"{\n"
" for(int i = 0; i < 4; i++)\n"
" {\n"
" color[i] = (i % 2 == 0) ? 0.0 : 1.0;\n"
" }\n"
" gl_Position = vec4(position.xy, 0.0, 1.0);\n"
"}\n";
const std::string fs =
"#version 300 es\n"
"precision mediump float;\n"
"in vec4 color;\n"
"out vec4 fragColor;\n"
"void main()\n"
"{\n"
" fragColor = color;\n"
"}\n";
const ProgramHandles ph = createProgram(vs, fs);
// Expect the info log to contain "unrolled". This is not a spec requirement.
GLsizei length = 0;
glGetShaderiv(ph.vertexShader, GL_INFO_LOG_LENGTH, &length);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
EXPECT_NE(length, 0);
char *log = new char[length];
GLsizei written = 0;
glGetShaderInfoLog(ph.vertexShader, length, &written, log);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
EXPECT_EQ(length, written + 1);
EXPECT_NE(strstr(log, "unrolled"), nullptr);
delete log;
glUseProgram(ph.program);
glClearColor(0.0, 0.0, 0.0, 0.0);
glClear(GL_COLOR_BUFFER_BIT);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
drawQuad(ph.program);
deleteProgram(ph);
compareColor(green);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
Uninitialize();
}
// Test non-canonical or non-deterministic loops do not get unrolled
TEST_F(SwiftShaderTest, DynamicLoop)
{
Initialize(3, false);
unsigned char green[4] = { 0, 255, 0, 255 };
const std::string vs =
"#version 300 es\n"
"in vec4 position;\n"
"out vec4 color;\n"
"void main()\n"
"{\n"
" for(int i = 0; i < 4; )\n"
" {\n"
" color[i] = (i % 2 == 0) ? 0.0 : 1.0;\n"
" i++;"
" }\n"
" gl_Position = vec4(position.xy, 0.0, 1.0);\n"
"}\n";
const std::string fs =
"#version 300 es\n"
"precision mediump float;\n"
"in vec4 color;\n"
"out vec4 fragColor;\n"
"void main()\n"
"{\n"
" vec4 temp;"
" for(int i = 0; i < 4; i++)\n"
" {\n"
" if(color.x < 0.0) return;"
" temp[i] = color[i];\n"
" }\n"
" fragColor = vec4(temp[0], temp[1], temp[2], temp[3]);\n"
"}\n";
const ProgramHandles ph = createProgram(vs, fs);
// Expect the info logs to be empty. This is not a spec requirement.
GLsizei length = 0;
glGetShaderiv(ph.vertexShader, GL_INFO_LOG_LENGTH, &length);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
EXPECT_EQ(length, 0);
glGetShaderiv(ph.fragmentShader, GL_INFO_LOG_LENGTH, &length);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
EXPECT_EQ(length, 0);
glUseProgram(ph.program);
glClearColor(0.0, 0.0, 0.0, 0.0);
glClear(GL_COLOR_BUFFER_BIT);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
drawQuad(ph.program);
deleteProgram(ph);
compareColor(green);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
Uninitialize();
}
// Test dynamic indexing
TEST_F(SwiftShaderTest, DynamicIndexing)
{
Initialize(3, false);
unsigned char green[4] = { 0, 255, 0, 255 };
const std::string vs =
"#version 300 es\n"
"in vec4 position;\n"
"out float color[4];\n"
"void main()\n"
"{\n"
" for(int i = 0; i < 4; )\n"
" {\n"
" int j = (gl_VertexID + i) % 4;\n"
" color[j] = (j % 2 == 0) ? 0.0 : 1.0;\n"
" i++;"
" }\n"
" gl_Position = vec4(position.xy, 0.0, 1.0);\n"
"}\n";
const std::string fs =
"#version 300 es\n"
"precision mediump float;\n"
"in float color[4];\n"
"out vec4 fragColor;\n"
"void main()\n"
"{\n"
" float temp[4];"
" for(int i = 0; i < 4; )\n"
" {\n"
" temp[i] = color[i];\n"
" i++;"
" }\n"
" fragColor = vec4(temp[0], temp[1], temp[2], temp[3]);\n"
"}\n";
const ProgramHandles ph = createProgram(vs, fs);
glUseProgram(ph.program);
glClearColor(0.0, 0.0, 0.0, 0.0);
glClear(GL_COLOR_BUFFER_BIT);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
drawQuad(ph.program);
deleteProgram(ph);
compareColor(green);
EXPECT_GLENUM_EQ(GL_NONE, glGetError());
Uninitialize();
}
// Test sampling from a sampler in a struct as a function argument
TEST_F(SwiftShaderTest, SamplerArrayInStructArrayAsFunctionArg)
{
......
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