Commit 716b2cba by Jamie Madill Committed by Commit Bot

Use bitset masks for active shader buffers.

This switches the tracking for the uniform, shader storage, and atomic counter buffers to use bitset masks to determine where there are active buffers. This will make iterating these buffer sets faster. Also renames the limit for atomic counter buffers to be consistent with the other buffer types. Also applies the implementation limit to atomic counter buffer bindings. This fixes out-of-bounds access on some Linux platforms that expose a large number of bindings. Bug: angleproject:5736 Change-Id: Ice801645697592d1dda6aebf0cb69767594cc0c5 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2757509 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarTim Van Patten <timvp@google.com>
parent 81dcf078
...@@ -37,14 +37,13 @@ enum ...@@ -37,14 +37,13 @@ enum
IMPLEMENTATION_MAX_GEOMETRY_SHADER_UNIFORM_BUFFERS = 16, IMPLEMENTATION_MAX_GEOMETRY_SHADER_UNIFORM_BUFFERS = 16,
IMPLEMENTATION_MAX_FRAGMENT_SHADER_UNIFORM_BUFFERS = 16, IMPLEMENTATION_MAX_FRAGMENT_SHADER_UNIFORM_BUFFERS = 16,
IMPLEMENTATION_MAX_COMPUTE_SHADER_UNIFORM_BUFFERS = 16, IMPLEMENTATION_MAX_COMPUTE_SHADER_UNIFORM_BUFFERS = 16,
// GL_EXT_geometry_shader increases the minimum value of GL_MAX_COMBINED_UNIFORM_BLOCKS to 36. // GL_EXT_geometry_shader increases the minimum value of GL_MAX_COMBINED_UNIFORM_BLOCKS to 36.
// GL_EXT_tessellation_shader increases the minimum value of GL_MAX_COMBINED_UNIFORM_BLOCKS // GL_EXT_tessellation_shader increases the minimum value to 60.
// to 60.
IMPLEMENTATION_MAX_COMBINED_SHADER_UNIFORM_BUFFERS = 60, IMPLEMENTATION_MAX_COMBINED_SHADER_UNIFORM_BUFFERS = 60,
// GL_EXT_geometry_shader increases the minimum value of GL_MAX_UNIFORM_BUFFER_BINDINGS to 48. // GL_EXT_geometry_shader increases the minimum value of GL_MAX_UNIFORM_BUFFER_BINDINGS to 48.
// Vulkan's minimum value for maxDescriptorSetUniformBuffers is 72 so allow exposing up to that // GL_EXT_tessellation_shader increases the minimum value to 72.
// many.
IMPLEMENTATION_MAX_UNIFORM_BUFFER_BINDINGS = 72, IMPLEMENTATION_MAX_UNIFORM_BUFFER_BINDINGS = 72,
// Transform feedback limits set to the minimum required by the spec. // Transform feedback limits set to the minimum required by the spec.
...@@ -77,7 +76,7 @@ enum ...@@ -77,7 +76,7 @@ enum
IMPLEMENTATION_MAX_IMAGE_UNITS = IMPLEMENTATION_MAX_ACTIVE_TEXTURES, IMPLEMENTATION_MAX_IMAGE_UNITS = IMPLEMENTATION_MAX_ACTIVE_TEXTURES,
// Maximum number of slots allocated for atomic counter buffers. // Maximum number of slots allocated for atomic counter buffers.
IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS = 8, IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS = 8,
// Implementation upper limits, real maximums depend on the hardware. // Implementation upper limits, real maximums depend on the hardware.
IMPLEMENTATION_MAX_SHADER_STORAGE_BUFFER_BINDINGS = 64, IMPLEMENTATION_MAX_SHADER_STORAGE_BUFFER_BINDINGS = 64,
......
...@@ -3689,10 +3689,12 @@ void Context::initCaps() ...@@ -3689,10 +3689,12 @@ void Context::initCaps()
for (ShaderType shaderType : AllShaderTypes()) for (ShaderType shaderType : AllShaderTypes())
{ {
ANGLE_LIMIT_CAP(mState.mCaps.maxShaderAtomicCounterBuffers[shaderType], ANGLE_LIMIT_CAP(mState.mCaps.maxShaderAtomicCounterBuffers[shaderType],
IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS); IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS);
} }
ANGLE_LIMIT_CAP(mState.mCaps.maxAtomicCounterBufferBindings,
IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS);
ANGLE_LIMIT_CAP(mState.mCaps.maxCombinedAtomicCounterBuffers, ANGLE_LIMIT_CAP(mState.mCaps.maxCombinedAtomicCounterBuffers,
IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS); IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS);
for (ShaderType shaderType : AllShaderTypes()) for (ShaderType shaderType : AllShaderTypes())
{ {
......
...@@ -347,7 +347,6 @@ State::State(const State *shareContextState, ...@@ -347,7 +347,6 @@ State::State(const State *shareContextState,
mProvokingVertex(gl::ProvokingVertexConvention::LastVertexConvention), mProvokingVertex(gl::ProvokingVertexConvention::LastVertexConvention),
mVertexArray(nullptr), mVertexArray(nullptr),
mActiveSampler(0), mActiveSampler(0),
mValidAtomicCounterBufferCount(0),
mPrimitiveRestart(false), mPrimitiveRestart(false),
mDebug(debug), mDebug(debug),
mMultiSampling(false), mMultiSampling(false),
...@@ -571,17 +570,19 @@ void State::reset(const Context *context) ...@@ -571,17 +570,19 @@ void State::reset(const Context *context)
{ {
UpdateIndexedBufferBinding(context, &buf, nullptr, BufferBinding::Uniform, 0, 0); UpdateIndexedBufferBinding(context, &buf, nullptr, BufferBinding::Uniform, 0, 0);
} }
mBoundUniformBuffersMask.reset();
for (OffsetBindingPointer<Buffer> &buf : mAtomicCounterBuffers) for (OffsetBindingPointer<Buffer> &buf : mAtomicCounterBuffers)
{ {
UpdateIndexedBufferBinding(context, &buf, nullptr, BufferBinding::AtomicCounter, 0, 0); UpdateIndexedBufferBinding(context, &buf, nullptr, BufferBinding::AtomicCounter, 0, 0);
} }
mValidAtomicCounterBufferCount = 0; mBoundAtomicCounterBuffersMask.reset();
for (OffsetBindingPointer<Buffer> &buf : mShaderStorageBuffers) for (OffsetBindingPointer<Buffer> &buf : mShaderStorageBuffers)
{ {
UpdateIndexedBufferBinding(context, &buf, nullptr, BufferBinding::ShaderStorage, 0, 0); UpdateIndexedBufferBinding(context, &buf, nullptr, BufferBinding::ShaderStorage, 0, 0);
} }
mBoundShaderStorageBuffersMask.reset();
mClipDistancesEnabled.reset(); mClipDistancesEnabled.reset();
...@@ -2045,27 +2046,17 @@ angle::Result State::setIndexedBufferBinding(const Context *context, ...@@ -2045,27 +2046,17 @@ angle::Result State::setIndexedBufferBinding(const Context *context,
setBufferBinding(context, target, buffer); setBufferBinding(context, target, buffer);
break; break;
case BufferBinding::Uniform: case BufferBinding::Uniform:
mBoundUniformBuffersMask.set(index, buffer != nullptr);
UpdateIndexedBufferBinding(context, &mUniformBuffers[index], buffer, target, offset, UpdateIndexedBufferBinding(context, &mUniformBuffers[index], buffer, target, offset,
size); size);
break; break;
case BufferBinding::AtomicCounter: case BufferBinding::AtomicCounter:
if (!mAtomicCounterBuffers[index].get() && buffer) mBoundAtomicCounterBuffersMask.set(index, buffer != nullptr);
{
// going from an invalid binding to a valid one, increment the count
mValidAtomicCounterBufferCount++;
ASSERT(mValidAtomicCounterBufferCount <=
static_cast<uint32_t>(getCaps().maxAtomicCounterBufferBindings));
}
else if (mAtomicCounterBuffers[index].get() && !buffer)
{
// going from a valid binding to an invalid one, decrement the count
mValidAtomicCounterBufferCount--;
ASSERT(mValidAtomicCounterBufferCount >= 0);
}
UpdateIndexedBufferBinding(context, &mAtomicCounterBuffers[index], buffer, target, UpdateIndexedBufferBinding(context, &mAtomicCounterBuffers[index], buffer, target,
offset, size); offset, size);
break; break;
case BufferBinding::ShaderStorage: case BufferBinding::ShaderStorage:
mBoundShaderStorageBuffersMask.set(index, buffer != nullptr);
UpdateIndexedBufferBinding(context, &mShaderStorageBuffers[index], buffer, target, UpdateIndexedBufferBinding(context, &mShaderStorageBuffers[index], buffer, target,
offset, size); offset, size);
break; break;
...@@ -2122,29 +2113,38 @@ angle::Result State::detachBuffer(Context *context, const Buffer *buffer) ...@@ -2122,29 +2113,38 @@ angle::Result State::detachBuffer(Context *context, const Buffer *buffer)
context->getStateCache().onVertexArrayStateChange(context); context->getStateCache().onVertexArrayStateChange(context);
} }
for (auto &buf : mUniformBuffers) for (size_t uniformBufferIndex : mBoundUniformBuffersMask)
{ {
if (buf.id() == bufferID) OffsetBindingPointer<Buffer> &binding = mUniformBuffers[uniformBufferIndex];
if (binding.id() == bufferID)
{ {
UpdateIndexedBufferBinding(context, &buf, nullptr, BufferBinding::Uniform, 0, 0); UpdateIndexedBufferBinding(context, &binding, nullptr, BufferBinding::Uniform, 0, 0);
mBoundUniformBuffersMask.reset(uniformBufferIndex);
} }
} }
for (auto &buf : mAtomicCounterBuffers) for (size_t atomicCounterBufferIndex : mBoundAtomicCounterBuffersMask)
{ {
if (buf.id() == bufferID) OffsetBindingPointer<Buffer> &binding = mAtomicCounterBuffers[atomicCounterBufferIndex];
if (binding.id() == bufferID)
{ {
UpdateIndexedBufferBinding(context, &buf, nullptr, BufferBinding::AtomicCounter, 0, 0); UpdateIndexedBufferBinding(context, &binding, nullptr, BufferBinding::AtomicCounter, 0,
mValidAtomicCounterBufferCount--; 0);
ASSERT(mValidAtomicCounterBufferCount >= 0); mBoundAtomicCounterBuffersMask.reset(atomicCounterBufferIndex);
} }
} }
for (auto &buf : mShaderStorageBuffers) for (size_t shaderStorageBufferIndex : mBoundShaderStorageBuffersMask)
{ {
if (buf.id() == bufferID) OffsetBindingPointer<Buffer> &binding = mShaderStorageBuffers[shaderStorageBufferIndex];
if (binding.id() == bufferID)
{ {
UpdateIndexedBufferBinding(context, &buf, nullptr, BufferBinding::ShaderStorage, 0, 0); UpdateIndexedBufferBinding(context, &binding, nullptr, BufferBinding::ShaderStorage, 0,
0);
mBoundShaderStorageBuffersMask.reset(shaderStorageBufferIndex);
} }
} }
......
...@@ -446,7 +446,7 @@ class State : angle::NonCopyable ...@@ -446,7 +446,7 @@ class State : angle::NonCopyable
ANGLE_INLINE bool hasValidAtomicCounterBuffer() const ANGLE_INLINE bool hasValidAtomicCounterBuffer() const
{ {
return mValidAtomicCounterBufferCount > 0; return mBoundAtomicCounterBuffersMask.any();
} }
const OffsetBindingPointer<Buffer> &getIndexedUniformBuffer(size_t index) const; const OffsetBindingPointer<Buffer> &getIndexedUniformBuffer(size_t index) const;
...@@ -1068,9 +1068,14 @@ class State : angle::NonCopyable ...@@ -1068,9 +1068,14 @@ class State : angle::NonCopyable
BufferVector mUniformBuffers; BufferVector mUniformBuffers;
BufferVector mAtomicCounterBuffers; BufferVector mAtomicCounterBuffers;
uint32_t mValidAtomicCounterBufferCount;
BufferVector mShaderStorageBuffers; BufferVector mShaderStorageBuffers;
angle::BitSet<gl::IMPLEMENTATION_MAX_UNIFORM_BUFFER_BINDINGS> mBoundUniformBuffersMask;
angle::BitSet<gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS>
mBoundAtomicCounterBuffersMask;
angle::BitSet<gl::IMPLEMENTATION_MAX_SHADER_STORAGE_BUFFER_BINDINGS>
mBoundShaderStorageBuffersMask;
BindingPointer<TransformFeedback> mTransformFeedback; BindingPointer<TransformFeedback> mTransformFeedback;
PixelUnpackState mUnpack; PixelUnpackState mUnpack;
......
...@@ -841,8 +841,8 @@ using UniformBuffersArray = std::array<T, IMPLEMENTATION_MAX_UNIFORM_BUFFER_BIND ...@@ -841,8 +841,8 @@ using UniformBuffersArray = std::array<T, IMPLEMENTATION_MAX_UNIFORM_BUFFER_BIND
template <typename T> template <typename T>
using StorageBuffersArray = std::array<T, IMPLEMENTATION_MAX_SHADER_STORAGE_BUFFER_BINDINGS>; using StorageBuffersArray = std::array<T, IMPLEMENTATION_MAX_SHADER_STORAGE_BUFFER_BINDINGS>;
template <typename T> template <typename T>
using AtomicCounterBuffersArray = std::array<T, IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS>; using AtomicCounterBuffersArray = std::array<T, IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS>;
using AtomicCounterBufferMask = angle::BitSet<IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS>; using AtomicCounterBufferMask = angle::BitSet<IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS>;
template <typename T> template <typename T>
using ImagesArray = std::array<T, IMPLEMENTATION_MAX_IMAGE_UNITS>; using ImagesArray = std::array<T, IMPLEMENTATION_MAX_IMAGE_UNITS>;
......
...@@ -1065,7 +1065,7 @@ std::unique_ptr<rx::LinkEvent> ProgramD3D::load(const gl::Context *context, ...@@ -1065,7 +1065,7 @@ std::unique_ptr<rx::LinkEvent> ProgramD3D::load(const gl::Context *context,
mImage2DUniforms.push_back(image2Duniform); mImage2DUniforms.push_back(image2Duniform);
} }
for (unsigned int ii = 0; ii < gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS; ++ii) for (unsigned int ii = 0; ii < gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS; ++ii)
{ {
unsigned int index = stream->readInt<unsigned int>(); unsigned int index = stream->readInt<unsigned int>();
mComputeAtomicCounterBufferRegisterIndices[ii] = index; mComputeAtomicCounterBufferRegisterIndices[ii] = index;
...@@ -1381,7 +1381,7 @@ void ProgramD3D::save(const gl::Context *context, gl::BinaryOutputStream *stream ...@@ -1381,7 +1381,7 @@ void ProgramD3D::save(const gl::Context *context, gl::BinaryOutputStream *stream
gl::WriteShaderVar(stream, image2DUniform); gl::WriteShaderVar(stream, image2DUniform);
} }
for (unsigned int ii = 0; ii < gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS; ++ii) for (unsigned int ii = 0; ii < gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS; ++ii)
{ {
stream->writeInt(mComputeAtomicCounterBufferRegisterIndices[ii]); stream->writeInt(mComputeAtomicCounterBufferRegisterIndices[ii]);
} }
......
...@@ -584,7 +584,7 @@ class ProgramD3D : public ProgramImpl ...@@ -584,7 +584,7 @@ class ProgramD3D : public ProgramImpl
std::map<std::string, int> mAtomicBindingMap; std::map<std::string, int> mAtomicBindingMap;
std::vector<D3DUniformBlock> mD3DUniformBlocks; std::vector<D3DUniformBlock> mD3DUniformBlocks;
std::vector<D3DInterfaceBlock> mD3DShaderStorageBlocks; std::vector<D3DInterfaceBlock> mD3DShaderStorageBlocks;
std::array<unsigned int, gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS> std::array<unsigned int, gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS>
mComputeAtomicCounterBufferRegisterIndices; mComputeAtomicCounterBufferRegisterIndices;
std::vector<sh::ShaderVariable> mImage2DUniforms; std::vector<sh::ShaderVariable> mImage2DUniforms;
......
...@@ -526,7 +526,7 @@ void ProgramExecutableVk::addAtomicCounterBufferDescriptorSetDesc( ...@@ -526,7 +526,7 @@ void ProgramExecutableVk::addAtomicCounterBufferDescriptorSetDesc(
// A single storage buffer array is used for all stages for simplicity. // A single storage buffer array is used for all stages for simplicity.
descOut->update(info.binding, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, descOut->update(info.binding, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER,
gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS, gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS,
gl_vk::kShaderStageMap[shaderType], nullptr); gl_vk::kShaderStageMap[shaderType], nullptr);
} }
......
...@@ -1774,7 +1774,8 @@ gl::Version RendererVk::getMaxSupportedESVersion() const ...@@ -1774,7 +1774,8 @@ gl::Version RendererVk::getMaxSupportedESVersion() const
// either none or IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS atomic counter buffers. So if // either none or IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS atomic counter buffers. So if
// Vulkan doesn't support at least that many storage buffers in compute, we don't support 3.1. // Vulkan doesn't support at least that many storage buffers in compute, we don't support 3.1.
const uint32_t kMinimumStorageBuffersForES31 = const uint32_t kMinimumStorageBuffersForES31 =
gl::limits::kMinimumComputeStorageBuffers + gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS; gl::limits::kMinimumComputeStorageBuffers +
gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS;
if (mPhysicalDeviceProperties.limits.maxPerStageDescriptorStorageBuffers < if (mPhysicalDeviceProperties.limits.maxPerStageDescriptorStorageBuffers <
kMinimumStorageBuffersForES31) kMinimumStorageBuffersForES31)
{ {
......
...@@ -694,20 +694,21 @@ void RendererVk::ensureCapsInitialized() const ...@@ -694,20 +694,21 @@ void RendererVk::ensureCapsInitialized() const
// likely not very useful, so we use the same limit (4 + MAX_ATOMIC_COUNTER_BUFFERS) for the // likely not very useful, so we use the same limit (4 + MAX_ATOMIC_COUNTER_BUFFERS) for the
// vertex stage to determine if we would want to add support for atomic counter buffers. // vertex stage to determine if we would want to add support for atomic counter buffers.
constexpr uint32_t kMinimumStorageBuffersForAtomicCounterBufferSupport = constexpr uint32_t kMinimumStorageBuffersForAtomicCounterBufferSupport =
gl::limits::kMinimumComputeStorageBuffers + gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS; gl::limits::kMinimumComputeStorageBuffers +
gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS;
uint32_t maxVertexStageAtomicCounterBuffers = 0; uint32_t maxVertexStageAtomicCounterBuffers = 0;
uint32_t maxPerStageAtomicCounterBuffers = 0; uint32_t maxPerStageAtomicCounterBuffers = 0;
uint32_t maxCombinedAtomicCounterBuffers = 0; uint32_t maxCombinedAtomicCounterBuffers = 0;
if (maxPerStageStorageBuffers >= kMinimumStorageBuffersForAtomicCounterBufferSupport) if (maxPerStageStorageBuffers >= kMinimumStorageBuffersForAtomicCounterBufferSupport)
{ {
maxPerStageAtomicCounterBuffers = gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS; maxPerStageAtomicCounterBuffers = gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS;
maxCombinedAtomicCounterBuffers = gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS; maxCombinedAtomicCounterBuffers = gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS;
} }
if (maxVertexStageStorageBuffers >= kMinimumStorageBuffersForAtomicCounterBufferSupport) if (maxVertexStageStorageBuffers >= kMinimumStorageBuffersForAtomicCounterBufferSupport)
{ {
maxVertexStageAtomicCounterBuffers = gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFERS; maxVertexStageAtomicCounterBuffers = gl::IMPLEMENTATION_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS;
} }
maxVertexStageStorageBuffers -= maxVertexStageAtomicCounterBuffers; maxVertexStageStorageBuffers -= maxVertexStageAtomicCounterBuffers;
......
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