Skip to content

A
angle
Commit e17b5ba5 by Luc Ferron Committed by Commit Bot
Options

Vulkan: Keep unused uniforms list to fix glslang issues

We we're unable to cleanup the unused uniforms if we did not keep a list of them while parsing them in ProgramLinkResource. Now that we keep a history of them, we're able to clean them up and fix a few dEQP tests. Bug: angleproject:2582 Bug: angleproject:2585 Bug: angleproject:2587 Bug: angleproject:2589 Bug: angleproject:2590 Bug: angleproject:2593 Change-Id: Ic1f9151e356a3d05e83f1031cc7b187b370284e5 Reviewed-on: https://chromium-review.googlesource.com/1085644 Commit-Queue: Luc Ferron <lucferron@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org>
parent 45b5a875
Showing with 120 additions and 67 deletions
src/libANGLE/Program.cpp
......@@ -1096,8 +1096,16 @@ Error Program::link(const gl::Context *context)
if (mState.mAttachedShaders[ShaderType::Compute])
{
ProgramLinkedResources resources = {
{0, PackMode::ANGLE_RELAXED},
{&mState.mUniformBlocks, &mState.mUniforms},
{&mState.mShaderStorageBlocks, &mState.mBufferVariables},
{&mState.mAtomicCounterBuffers},
{}};
GLuint combinedImageUniforms = 0u;
if (!linkUniforms(context, mInfoLog, mUniformLocationBindings, &combinedImageUniforms))
if (!linkUniforms(context, mInfoLog, mUniformLocationBindings, &combinedImageUniforms,
&resources.unusedUniforms))
{
return NoError();
}
......@@ -1124,12 +1132,6 @@ Error Program::link(const gl::Context *context)
return NoError();
}
ProgramLinkedResources resources = {
{0, PackMode::ANGLE_RELAXED},
{&mState.mUniformBlocks, &mState.mUniforms},
{&mState.mShaderStorageBlocks, &mState.mBufferVariables},
{&mState.mAtomicCounterBuffers}};
InitUniformBlockLinker(context, mState, &resources.uniformBlockLinker);
InitShaderStorageBlockLinker(context, mState, &resources.shaderStorageBlockLinker);
......@@ -1141,6 +1143,26 @@ Error Program::link(const gl::Context *context)
}
else
{
// Map the varyings to the register file
// In WebGL, we use a slightly different handling for packing variables.
gl::PackMode packMode = PackMode::ANGLE_RELAXED;
if (data.getLimitations().noFlexibleVaryingPacking)
{
// D3D9 pack mode is strictly more strict than WebGL, so takes priority.
packMode = PackMode::ANGLE_NON_CONFORMANT_D3D9;
}
else if (data.getExtensions().webglCompatibility)
{
packMode = PackMode::WEBGL_STRICT;
}
ProgramLinkedResources resources = {
{data.getCaps().maxVaryingVectors, packMode},
{&mState.mUniformBlocks, &mState.mUniforms},
{&mState.mShaderStorageBlocks, &mState.mBufferVariables},
{&mState.mAtomicCounterBuffers},
{}};
if (!linkAttributes(context, mInfoLog))
{
return NoError();
......@@ -1152,7 +1174,8 @@ Error Program::link(const gl::Context *context)
}
GLuint combinedImageUniforms = 0u;
if (!linkUniforms(context, mInfoLog, mUniformLocationBindings, &combinedImageUniforms))
if (!linkUniforms(context, mInfoLog, mUniformLocationBindings, &combinedImageUniforms,
&resources.unusedUniforms))
{
return NoError();
}
......@@ -1178,25 +1201,6 @@ Error Program::link(const gl::Context *context)
ASSERT(mState.mAttachedShaders[ShaderType::Vertex]);
mState.mNumViews = mState.mAttachedShaders[ShaderType::Vertex]->getNumViews(context);
// Map the varyings to the register file
// In WebGL, we use a slightly different handling for packing variables.
gl::PackMode packMode = PackMode::ANGLE_RELAXED;
if (data.getLimitations().noFlexibleVaryingPacking)
{
// D3D9 pack mode is strictly more strict than WebGL, so takes priority.
packMode = PackMode::ANGLE_NON_CONFORMANT_D3D9;
}
else if (data.getExtensions().webglCompatibility)
{
packMode = PackMode::WEBGL_STRICT;
}
ProgramLinkedResources resources = {
{data.getCaps().maxVaryingVectors, packMode},
{&mState.mUniformBlocks, &mState.mUniforms},
{&mState.mShaderStorageBlocks, &mState.mBufferVariables},
{&mState.mAtomicCounterBuffers}};
InitUniformBlockLinker(context, mState, &resources.uniformBlockLinker);
InitShaderStorageBlockLinker(context, mState, &resources.shaderStorageBlockLinker);
......@@ -2558,7 +2562,8 @@ bool Program::linkValidateFragmentInputBindings(const Context *context, gl::Info
bool Program::linkUniforms(const Context *context,
InfoLog &infoLog,
const ProgramBindings &uniformLocationBindings,
GLuint *combinedImageUniformsCount)
GLuint *combinedImageUniformsCount,
std::vector<UnusedUniform> *unusedUniforms)
{
UniformLinker linker(mState);
if (!linker.link(context, infoLog, uniformLocationBindings))
......@@ -2566,7 +2571,7 @@ bool Program::linkUniforms(const Context *context,
return false;
}
linker.getResults(&mState.mUniforms, &mState.mUniformLocations);
linker.getResults(&mState.mUniforms, unusedUniforms, &mState.mUniformLocations);
linkSamplerAndImageBindings(combinedImageUniformsCount);
......
src/libANGLE/Program.h
......@@ -40,6 +40,7 @@ struct TranslatedAttribute;
namespace gl
{
struct UnusedUniform;
struct Caps;
class Context;
class ContextState;
......@@ -747,7 +748,8 @@ class Program final : angle::NonCopyable, public LabeledObject
bool linkUniforms(const Context *context,
InfoLog &infoLog,
const ProgramBindings &uniformLocationBindings,
GLuint *combinedImageUniformsCount);
GLuint *combinedImageUniformsCount,
std::vector<UnusedUniform> *unusedUniforms);
void linkSamplerAndImageBindings(GLuint *combinedImageUniformsCount);
bool linkAtomicCounterBuffers();
......
src/libANGLE/ProgramLinkedResources.cpp
......@@ -253,9 +253,11 @@ UniformLinker::UniformLinker(const ProgramState &state) : mState(state)
UniformLinker::~UniformLinker() = default;
void UniformLinker::getResults(std::vector<LinkedUniform> *uniforms,
std::vector<UnusedUniform> *unusedUniforms,
std::vector<VariableLocation> *uniformLocations)
{
uniforms->swap(mUniforms);
unusedUniforms->swap(mUnusedUniforms);
uniformLocations->swap(mUniformLocations);
}
......@@ -501,6 +503,7 @@ void UniformLinker::pruneUnusedUniforms()
}
else
{
mUnusedUniforms.emplace_back(uniformIter->name, uniformIter->isSampler());
uniformIter = mUniforms.erase(uniformIter);
}
}
......@@ -513,13 +516,15 @@ bool UniformLinker::flattenUniformsAndCheckCapsForShader(
std::vector<LinkedUniform> &samplerUniforms,
std::vector<LinkedUniform> &imageUniforms,
std::vector<LinkedUniform> &atomicCounterUniforms,
std::vector<UnusedUniform> &unusedUniforms,
InfoLog &infoLog)
{
ShaderUniformCount shaderUniformCount;
for (const sh::Uniform &uniform : shader->getUniforms(context))
{
shaderUniformCount += flattenUniform(uniform, &samplerUniforms, &imageUniforms,
&atomicCounterUniforms, shader->getType());
shaderUniformCount +=
flattenUniform(uniform, &samplerUniforms, &imageUniforms, &atomicCounterUniforms,
&unusedUniforms, shader->getType());
}
ShaderType shaderType = shader->getType();
......@@ -573,6 +578,7 @@ bool UniformLinker::flattenUniformsAndCheckCaps(const Context *context, InfoLog
std::vector<LinkedUniform> samplerUniforms;
std::vector<LinkedUniform> imageUniforms;
std::vector<LinkedUniform> atomicCounterUniforms;
std::vector<UnusedUniform> unusedUniforms;
const Caps &caps = context->getCaps();
for (ShaderType shaderType : AllShaderTypes())
......@@ -584,7 +590,8 @@ bool UniformLinker::flattenUniformsAndCheckCaps(const Context *context, InfoLog
}
if (!flattenUniformsAndCheckCapsForShader(context, shader, caps, samplerUniforms,
imageUniforms, atomicCounterUniforms, infoLog))
imageUniforms, atomicCounterUniforms,
unusedUniforms, infoLog))
{
return false;
}
......@@ -593,6 +600,7 @@ bool UniformLinker::flattenUniformsAndCheckCaps(const Context *context, InfoLog
mUniforms.insert(mUniforms.end(), samplerUniforms.begin(), samplerUniforms.end());
mUniforms.insert(mUniforms.end(), imageUniforms.begin(), imageUniforms.end());
mUniforms.insert(mUniforms.end(), atomicCounterUniforms.begin(), atomicCounterUniforms.end());
mUnusedUniforms.insert(mUnusedUniforms.end(), unusedUniforms.begin(), unusedUniforms.end());
return true;
}
......@@ -601,17 +609,22 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenUniform(
std::vector<LinkedUniform> *samplerUniforms,
std::vector<LinkedUniform> *imageUniforms,
std::vector<LinkedUniform> *atomicCounterUniforms,
std::vector<UnusedUniform> *unusedUniforms,
ShaderType shaderType)
{
int location = uniform.location;
ShaderUniformCount shaderUniformCount =
flattenUniformImpl(uniform, uniform.name, uniform.mappedName, samplerUniforms,
imageUniforms, atomicCounterUniforms, shaderType, uniform.active,
uniform.staticUse, uniform.binding, uniform.offset, &location);
int location = uniform.location;
ShaderUniformCount shaderUniformCount = flattenUniformImpl(
uniform, uniform.name, uniform.mappedName, samplerUniforms, imageUniforms,
atomicCounterUniforms, unusedUniforms, shaderType, uniform.active, uniform.staticUse,
uniform.binding, uniform.offset, &location);
if (uniform.active)
{
return shaderUniformCount;
}
else
{
unusedUniforms->emplace_back(uniform.name, IsSamplerType(uniform.type));
}
return ShaderUniformCount();
}
......@@ -623,6 +636,7 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenArrayOfStructsUniform(
std::vector<LinkedUniform> *samplerUniforms,
std::vector<LinkedUniform> *imageUniforms,
std::vector<LinkedUniform> *atomicCounterUniforms,
std::vector<UnusedUniform> *unusedUniforms,
ShaderType shaderType,
bool markActive,
bool markStaticUse,
......@@ -642,15 +656,15 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenArrayOfStructsUniform(
{
shaderUniformCount += flattenArrayOfStructsUniform(
uniform, arrayNestingIndex + 1u, elementName, elementMappedName, samplerUniforms,
imageUniforms, atomicCounterUniforms, shaderType, markActive, markStaticUse,
binding, offset, location);
imageUniforms, atomicCounterUniforms, unusedUniforms, shaderType, markActive,
markStaticUse, binding, offset, location);
}
else
{
shaderUniformCount += flattenStructUniform(
uniform.fields, elementName, elementMappedName, samplerUniforms, imageUniforms,
atomicCounterUniforms, shaderType, markActive, markStaticUse, binding, offset,
location);
atomicCounterUniforms, unusedUniforms, shaderType, markActive, markStaticUse,
binding, offset, location);
}
}
return shaderUniformCount;
......@@ -663,6 +677,7 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenStructUniform(
std::vector<LinkedUniform> *samplerUniforms,
std::vector<LinkedUniform> *imageUniforms,
std::vector<LinkedUniform> *atomicCounterUniforms,
std::vector<UnusedUniform> *unusedUniforms,
ShaderType shaderType,
bool markActive,
bool markStaticUse,
......@@ -676,9 +691,10 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenStructUniform(
const std::string &fieldName = namePrefix + "." + field.name;
const std::string &fieldMappedName = mappedNamePrefix + "." + field.mappedName;
shaderUniformCount += flattenUniformImpl(field, fieldName, fieldMappedName, samplerUniforms,
imageUniforms, atomicCounterUniforms, shaderType,
markActive, markStaticUse, -1, -1, location);
shaderUniformCount +=
flattenUniformImpl(field, fieldName, fieldMappedName, samplerUniforms, imageUniforms,
atomicCounterUniforms, unusedUniforms, shaderType, markActive,
markStaticUse, -1, -1, location);
}
return shaderUniformCount;
}
......@@ -690,6 +706,7 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenArrayUniform(
std::vector<LinkedUniform> *samplerUniforms,
std::vector<LinkedUniform> *imageUniforms,
std::vector<LinkedUniform> *atomicCounterUniforms,
std::vector<UnusedUniform> *unusedUniforms,
ShaderType shaderType,
bool markActive,
bool markStaticUse,
......@@ -710,8 +727,8 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenArrayUniform(
shaderUniformCount +=
flattenUniformImpl(uniformElement, elementName, elementMappedName, samplerUniforms,
imageUniforms, atomicCounterUniforms, shaderType, markActive,
markStaticUse, binding, offset, location);
imageUniforms, atomicCounterUniforms, unusedUniforms, shaderType,
markActive, markStaticUse, binding, offset, location);
}
return shaderUniformCount;
}
......@@ -723,6 +740,7 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenUniformImpl(
std::vector<LinkedUniform> *samplerUniforms,
std::vector<LinkedUniform> *imageUniforms,
std::vector<LinkedUniform> *atomicCounterUniforms,
std::vector<UnusedUniform> *unusedUniforms,
ShaderType shaderType,
bool markActive,
bool markStaticUse,
......@@ -737,17 +755,17 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenUniformImpl(
{
if (uniform.isArray())
{
shaderUniformCount +=
flattenArrayOfStructsUniform(uniform, 0u, fullName, fullMappedName, samplerUniforms,
imageUniforms, atomicCounterUniforms, shaderType,
markActive, markStaticUse, binding, offset, location);
shaderUniformCount += flattenArrayOfStructsUniform(
uniform, 0u, fullName, fullMappedName, samplerUniforms, imageUniforms,
atomicCounterUniforms, unusedUniforms, shaderType, markActive, markStaticUse,
binding, offset, location);
}
else
{
shaderUniformCount +=
flattenStructUniform(uniform.fields, fullName, fullMappedName, samplerUniforms,
imageUniforms, atomicCounterUniforms, shaderType, markActive,
markStaticUse, binding, offset, location);
shaderUniformCount += flattenStructUniform(
uniform.fields, fullName, fullMappedName, samplerUniforms, imageUniforms,
atomicCounterUniforms, unusedUniforms, shaderType, markActive, markStaticUse,
binding, offset, location);
}
return shaderUniformCount;
}
......@@ -757,8 +775,8 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenUniformImpl(
// "For an active variable declared as an array of an aggregate data type (structures or
// arrays), a separate entry will be generated for each active array element"
return flattenArrayUniform(uniform, fullName, fullMappedName, samplerUniforms,
imageUniforms, atomicCounterUniforms, shaderType, markActive,
markStaticUse, binding, offset, location);
imageUniforms, atomicCounterUniforms, unusedUniforms, shaderType,
markActive, markStaticUse, binding, offset, location);
}
// Not a struct
......@@ -829,6 +847,10 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenUniformImpl(
{
linkedUniform.setActive(shaderType, true);
}
else
{
unusedUniforms->emplace_back(linkedUniform.name, linkedUniform.isSampler());
}
uniformList->push_back(linkedUniform);
}
......
src/libANGLE/ProgramLinkedResources.h
......@@ -38,8 +38,9 @@ struct LinkedUniform;
class ProgramState;
class ProgramBindings;
class Shader;
struct VariableLocation;
struct ShaderVariableBuffer;
struct UnusedUniform;
struct VariableLocation;
using AtomicCounterBuffer = ShaderVariableBuffer;
......@@ -54,6 +55,7 @@ class UniformLinker final : angle::NonCopyable
const ProgramBindings &uniformLocationBindings);
void getResults(std::vector<LinkedUniform> *uniforms,
std::vector<UnusedUniform> *unusedUniforms,
std::vector<VariableLocation> *uniformLocations);
private:
......@@ -88,6 +90,7 @@ class UniformLinker final : angle::NonCopyable
std::vector<LinkedUniform> &samplerUniforms,
std::vector<LinkedUniform> &imageUniforms,
std::vector<LinkedUniform> &atomicCounterUniforms,
std::vector<UnusedUniform> &unusedUniforms,
InfoLog &infoLog);
bool flattenUniformsAndCheckCaps(const Context *context, InfoLog &infoLog);
......@@ -97,6 +100,7 @@ class UniformLinker final : angle::NonCopyable
std::vector<LinkedUniform> *samplerUniforms,
std::vector<LinkedUniform> *imageUniforms,
std::vector<LinkedUniform> *atomicCounterUniforms,
std::vector<UnusedUniform> *unusedUniforms,
ShaderType shaderType);
ShaderUniformCount flattenArrayOfStructsUniform(
......@@ -107,7 +111,7 @@ class UniformLinker final : angle::NonCopyable
std::vector<LinkedUniform> *samplerUniforms,
std::vector<LinkedUniform> *imageUniforms,
std::vector<LinkedUniform> *atomicCounterUniforms,
std::vector<UnusedUniform> *unusedUniforms,
ShaderType shaderType,
bool markActive,
bool markStaticUse,
......@@ -121,6 +125,7 @@ class UniformLinker final : angle::NonCopyable
std::vector<LinkedUniform> *samplerUniforms,
std::vector<LinkedUniform> *imageUniforms,
std::vector<LinkedUniform> *atomicCounterUniforms,
std::vector<UnusedUniform> *unusedUniforms,
ShaderType shaderType,
bool markActive,
bool markStaticUse,
......@@ -134,6 +139,7 @@ class UniformLinker final : angle::NonCopyable
std::vector<LinkedUniform> *samplerUniforms,
std::vector<LinkedUniform> *imageUniforms,
std::vector<LinkedUniform> *atomicCounterUniforms,
std::vector<UnusedUniform> *unusedUniforms,
ShaderType shaderType,
bool markActive,
bool markStaticUse,
......@@ -149,6 +155,7 @@ class UniformLinker final : angle::NonCopyable
std::vector<LinkedUniform> *samplerUniforms,
std::vector<LinkedUniform> *imageUniforms,
std::vector<LinkedUniform> *atomicCounterUniforms,
std::vector<UnusedUniform> *unusedUniforms,
ShaderType shaderType,
bool markActive,
bool markStaticUse,
......@@ -165,6 +172,7 @@ class UniformLinker final : angle::NonCopyable
const ProgramState &mState;
std::vector<LinkedUniform> mUniforms;
std::vector<UnusedUniform> mUnusedUniforms;
std::vector<VariableLocation> mUniformLocations;
};
......@@ -303,12 +311,25 @@ class AtomicCounterBufferLinker final : angle::NonCopyable
// The link operation is responsible for finishing the link of uniform and interface blocks.
// This way it can filter out unreferenced resources and still have access to the info.
// TODO(jmadill): Integrate uniform linking/filtering as well as interface blocks.
struct UnusedUniform
{
UnusedUniform(std::string name, bool isSampler)
{
this->name = name;
this->isSampler = isSampler;
}
std::string name;
bool isSampler;
};
struct ProgramLinkedResources
{
VaryingPacking varyingPacking;
UniformBlockLinker uniformBlockLinker;
ShaderStorageBlockLinker shaderStorageBlockLinker;
AtomicCounterBufferLinker atomicCounterBufferLinker;
std::vector<UnusedUniform> unusedUniforms;
};
} // namespace gl
......
src/libANGLE/renderer/vulkan/GlslangWrapper.cpp
......@@ -41,7 +41,7 @@ void InsertLayoutSpecifierString(std::string *shaderString,
searchStringBuilder << kLayoutMarkerBegin << variableName << kMarkerEnd;
std::string searchString = searchStringBuilder.str();
if (layoutString != "")
if (!layoutString.empty())
{
angle::ReplaceSubstring(shaderString, searchString, "layout(" + layoutString + ")");
}
......@@ -193,7 +193,6 @@ gl::LinkResult GlslangWrapper::linkProgram(const gl::Context *glContext,
for (unsigned int uniformIndex : programState.getSamplerUniformRange())
{
const gl::LinkedUniform &samplerUniform = uniforms[uniformIndex];
std::string setBindingString = "set = 1, binding = " + Str(textureCount);
ASSERT(samplerUniform.isActive(gl::ShaderType::Vertex) ||
......@@ -221,6 +220,16 @@ gl::LinkResult GlslangWrapper::linkProgram(const gl::Context *glContext,
textureCount += samplerUniform.getBasicTypeElementCount();
}
for (const gl::UnusedUniform &unusedUniform : resources.unusedUniforms)
{
InsertLayoutSpecifierString(&vertexSource, unusedUniform.name, "");
InsertLayoutSpecifierString(&fragmentSource, unusedUniform.name, "");
std::string qualifierToUse = unusedUniform.isSampler ? kUniformQualifier : "";
InsertQualifierSpecifierString(&vertexSource, unusedUniform.name, qualifierToUse);
InsertQualifierSpecifierString(&fragmentSource, unusedUniform.name, qualifierToUse);
}
std::array<const char *, 2> strings = {{vertexSource.c_str(), fragmentSource.c_str()}};
std::array<int, 2> lengths = {
{static_cast<int>(vertexSource.length()), static_cast<int>(fragmentSource.length())}};
......
src/tests/deqp_support/deqp_gles2_test_expectations.txt
......@@ -216,12 +216,6 @@
2580 VULKAN : dEQP-GLES2.functional.buffer.write.recreate_store.different_size = SKIP
2580 VULKAN : dEQP-GLES2.functional.buffer.write.recreate_store.random_* = SKIP
2580 VULKAN : dEQP-GLES2.functional.buffer.write.random.* = SKIP
2582 VULKAN : dEQP-GLES2.functional.shaders.linkage.uniform_struct_fragment* = SKIP
2585 VULKAN : dEQP-GLES2.functional.shaders.qualification_order.variables.valid.* = SKIP
2587 VULKAN : dEQP-GLES2.functional.shaders.loops.* = SKIP
2589 VULKAN : dEQP-GLES2.functional.shaders.return.output_write* = SKIP
2589 VULKAN : dEQP-GLES2.functional.shaders.return.return_in_static_loop* = SKIP
2590 VULKAN : dEQP-GLES2.functional.shaders.discard.* = SKIP
2494 VULKAN : dEQP-GLES2.functional.shaders.struct.uniform.sampler_* = SKIP
2592 VULKAN : dEQP-GLES2.functional.shaders.builtin_variable.depth_range* = SKIP
2592 VULKAN : dEQP-GLES2.functional.shaders.builtin_variable.pointcoord = SKIP
......
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