Add padding to VkBuffer size to avoid access violation
By adding 15 bytes of padding, any vertex attribute within the buffer
can be read using accesses that are up to 16 bytes wide. This helps
performance by avoiding having to break up some accesses into multiple
smaller ones, and avoids code bloat.
Note that we detect when numerical overflow happens, which would
typically only occur in tests that want to force an allocation that
can't succeed, using a buffer of size 0xFFFFFFFFFFFFFFFF. We don't
test against vk::MAX_MEMORY_ALLOCATION_SIZE or PHYSICAL_DEVICE_HEAP_SIZE
because that would silently avoid valid usage violations such as
VUID-vkAllocateMemory-pAllocateInfo-01713.
This change also temporarily increases the memory heap size by 15 bytes,
to work around an ANGLE bug.
Bug: b/201282910
Bug: angleproject:6444
Change-Id: I9489c0a1e51e9986f49a4b25d3aca3858778f47a
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/57568
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/Vulkan/VkBuffer.cpp b/src/Vulkan/VkBuffer.cpp
index f19785e..f913942 100644
--- a/src/Vulkan/VkBuffer.cpp
+++ b/src/Vulkan/VkBuffer.cpp
@@ -13,10 +13,12 @@
// limitations under the License.
#include "VkBuffer.hpp"
+
#include "VkConfig.hpp"
#include "VkDeviceMemory.hpp"
#include <cstring>
+#include <limits>
namespace vk {
@@ -57,6 +59,17 @@
const VkMemoryRequirements Buffer::getMemoryRequirements() const
{
VkMemoryRequirements memoryRequirements = {};
+
+ // Add 15 bytes of padding to ensure that any type of attribute within the
+ // buffer can be read using 16-bit accesses.
+ // TODO(b/196822081): Also reserve space for a header containing the size of the buffer (for robust buffer access)
+ memoryRequirements.size = this->size + 15;
+
+ if(memoryRequirements.size < this->size) // Overflow occurred
+ {
+ memoryRequirements.size = std::numeric_limits<VkDeviceSize>::max();
+ }
+
if(usage & (VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT | VK_BUFFER_USAGE_UNIFORM_TEXEL_BUFFER_BIT))
{
memoryRequirements.alignment = vk::MIN_TEXEL_BUFFER_OFFSET_ALIGNMENT;
@@ -73,9 +86,9 @@
{
memoryRequirements.alignment = REQUIRED_MEMORY_ALIGNMENT;
}
+
memoryRequirements.memoryTypeBits = vk::MEMORY_TYPE_GENERIC_BIT;
- memoryRequirements.size = size; // TODO: also reserve space for a header containing
- // the size of the buffer (for robust buffer access)
+
return memoryRequirements;
}
diff --git a/src/Vulkan/VkConfig.hpp b/src/Vulkan/VkConfig.hpp
index fd461fe..d16e2f2 100644
--- a/src/Vulkan/VkConfig.hpp
+++ b/src/Vulkan/VkConfig.hpp
@@ -74,7 +74,10 @@
constexpr float SUBPIXEL_PRECISION_FACTOR = static_cast<float>(1 << SUBPIXEL_PRECISION_BITS);
constexpr int SUBPIXEL_PRECISION_MASK = 0xFFFFFFFF >> (32 - SUBPIXEL_PRECISION_BITS);
-constexpr VkDeviceSize MAX_MEMORY_ALLOCATION_SIZE = 0x40000000ull; // 0x40000000 = 1 GiB
+// TODO: The heap size should be configured based on available RAM.
+// FIXME(angleproject:6444): Remove the 15 bytes of padding.
+constexpr VkDeviceSize PHYSICAL_DEVICE_HEAP_SIZE = 0x80000000ull + 15; // 0x80000000 = 2 GiB
+constexpr VkDeviceSize MAX_MEMORY_ALLOCATION_SIZE = 0x40000000ull; // 0x40000000 = 1 GiB
// Memory offset calculations in 32-bit SIMD elements limit us to addressing at most 4 GiB.
// Signed arithmetic further restricts it to 2 GiB.
diff --git a/src/Vulkan/VkPhysicalDevice.cpp b/src/Vulkan/VkPhysicalDevice.cpp
index 9437356..f66c1c9 100644
--- a/src/Vulkan/VkPhysicalDevice.cpp
+++ b/src/Vulkan/VkPhysicalDevice.cpp
@@ -1663,7 +1663,7 @@
1, // memoryHeapCount
{
{
- 1ull << 31, // size, FIXME(sugoi): This should be configurable based on available RAM
+ vk::PHYSICAL_DEVICE_HEAP_SIZE, // size
VK_MEMORY_HEAP_DEVICE_LOCAL_BIT // flags
},
}
