Commit 763cec07 by Jamie Madill Committed by Commit Bot

Vulkan: Add warnings for cache struct packing.

This adds diagnostic warnings for packed cache structures. It ensures the packed versions of the structures don't have any unexpected misalignments or inserted members. This gives us consistent behaviour and ensures all memory is initialized. Implemented for Clang/GCC/MSVC. Bug: angleproject:2522 Change-Id: I6ec453a40d292e4a498319ffa767988a502d225e Reviewed-on: https://chromium-review.googlesource.com/c/1302533Reviewed-by: 's avatarFrank Henigman <fjhenigman@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
parent af9dd608
...@@ -317,4 +317,23 @@ std::ostream &FmtHex(std::ostream &os, T value) ...@@ -317,4 +317,23 @@ std::ostream &FmtHex(std::ostream &os, T value)
#define ANGLE_FUNCTION __func__ #define ANGLE_FUNCTION __func__
#endif #endif
// Defining ANGLE_ENABLE_STRUCT_PADDING_WARNINGS will enable warnings when members are added to
// structs to enforce packing. This is helpful for diagnosing unexpected struct sizes when making
// fast cache variables.
#if defined(__clang__)
#define ANGLE_ENABLE_STRUCT_PADDING_WARNINGS \
_Pragma("clang diagnostic push") _Pragma("clang diagnostic error \"-Wpadded\"")
#define ANGLE_DISABLE_STRUCT_PADDING_WARNINGS _Pragma("clang diagnostic pop")
#elif defined(COMPILER_GCC)
#define ANGLE_ENABLE_STRUCT_PADDING_WARNINGS \
_Pragma("GCC diagnostic push") _Pragma("GCC diagnostic error \"-Wpadded\"")
#define ANGLE_DISABLE_STRUCT_PADDING_WARNINGS _Pragma("GCC diagnostic pop")
#elif defined(_MSC_VER)
#define ANGLE_ENABLE_STRUCT_PADDING_WARNINGS __pragma(warning(push)) __pragma(warning(error : 4820))
#define ANGLE_DISABLE_STRUCT_PADDING_WARNINGS __pragma(warning(pop))
#else
#define ANGLE_ENABLE_STRUCT_PADDING_WARNINGS
#define ANGLE_DISABLE_STRUCT_PADDING_WARNINGS
#endif
#endif // COMMON_DEBUG_H_ #endif // COMMON_DEBUG_H_
...@@ -45,6 +45,9 @@ using SharedPipelineLayout = RefCounted<PipelineLayout>; ...@@ -45,6 +45,9 @@ using SharedPipelineLayout = RefCounted<PipelineLayout>;
// packing nicely into the desired space. This is something we could also potentially fix // packing nicely into the desired space. This is something we could also potentially fix
// with a redesign to use bitfields or bit mask operations. // with a redesign to use bitfields or bit mask operations.
// Enable struct padding warnings for the code below since it is used in caches.
ANGLE_ENABLE_STRUCT_PADDING_WARNINGS
struct alignas(4) PackedAttachmentDesc struct alignas(4) PackedAttachmentDesc
{ {
uint8_t flags; uint8_t flags;
...@@ -236,10 +239,11 @@ static_assert(sizeof(PackedColorBlendAttachmentState) == 8, "Size check failed") ...@@ -236,10 +239,11 @@ static_assert(sizeof(PackedColorBlendAttachmentState) == 8, "Size check failed")
struct PackedColorBlendStateInfo final struct PackedColorBlendStateInfo final
{ {
// Padded to round the strut size. // Padded to round the struct size.
uint32_t logicOpEnable; uint32_t logicOpEnable;
uint32_t logicOp; uint32_t logicOp;
uint32_t attachmentCount; uint32_t attachmentCount;
uint32_t padding;
float blendConstants[4]; float blendConstants[4];
PackedColorBlendAttachmentState attachments[gl::IMPLEMENTATION_MAX_DRAW_BUFFERS]; PackedColorBlendAttachmentState attachments[gl::IMPLEMENTATION_MAX_DRAW_BUFFERS];
}; };
...@@ -459,6 +463,9 @@ static_assert(sizeof(PipelineLayoutDesc) == ...@@ -459,6 +463,9 @@ static_assert(sizeof(PipelineLayoutDesc) ==
(sizeof(DescriptorSetLayoutArray<DescriptorSetLayoutDesc>) + (sizeof(DescriptorSetLayoutArray<DescriptorSetLayoutDesc>) +
sizeof(std::array<PackedPushConstantRange, kMaxPushConstantRanges>)), sizeof(std::array<PackedPushConstantRange, kMaxPushConstantRanges>)),
"Unexpected Size"); "Unexpected Size");
// Disable warnings about struct padding.
ANGLE_DISABLE_STRUCT_PADDING_WARNINGS
} // namespace vk } // namespace vk
} // namespace rx } // namespace rx
......
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