Refactor GLES routine cache keys to use Memset<T>

src/Renderer/Blitter.hpp

@@ -53,13 +53,14 @@ namespace sw
 			bool clampToEdge : 1;
 		};
 
-		struct State : Options
+		struct State : Memset<State>, Options
 		{
-			State() = default;
-			State(const Options &options) : Options(options) {}
+			State() : Memset(this, 0) {}
+			State(const Options &options) : Memset(this, 0), Options(options) {}
 
 			bool operator==(const State &state) const
 			{
+				static_assert(is_memcmparable<State>::value, "Cannot memcmp State");
 				return memcmp(this, &state, sizeof(State)) == 0;
 			}
 

src/Renderer/LRUCache.hpp

@@ -17,6 +17,9 @@
 
 #include "Common/Math.hpp"
 
+#include <cstring>
+#include <type_traits>
+
 namespace sw
 {
 	template<class Key, class Data>
@@ -43,6 +46,46 @@ namespace sw
 		Key **ref;
 		Data *data;
 	};
+
+	// Helper class for clearing the memory of objects at construction.
+	// Useful as the first base class of cache keys which may contain padding bytes or bits otherwise left uninitialized.
+	template<class T>
+	struct Memset
+	{
+		Memset(T *object, int val)
+		{
+			static_assert(std::is_base_of<Memset<T>, T>::value, "Memset<T> must only clear the memory of a type of which it is a base class");
+
+			// GCC 8+ warns that
+			// "‘void* memset(void*, int, size_t)’ clearing an object of non-trivial type ‘T’;
+			//  use assignment or value-initialization instead [-Werror=class-memaccess]"
+			// This is benign iff it happens before any of the base or member constructrs are called.
+			#if defined(__GNUC__) && (__GNUC__ >= 8)
+			#pragma GCC diagnostic push
+			#pragma GCC diagnostic ignored "-Wclass-memaccess"
+			#endif
+
+			memset(object, 0, sizeof(T));
+
+			#if defined(__GNUC__) && (__GNUC__ >= 8)
+			#pragma GCC diagnostic pop
+			#endif
+		}
+	};
+
+	// Traits-like helper class for checking if objects can be compared using memcmp().
+	// Useful for statically asserting if a cache key can implement operator==() with memcmp().
+	template<typename T>
+	struct is_memcmparable
+	{
+		// std::is_trivially_copyable is not available in older GCC versions.
+		#if !defined(__GNUC__) || __GNUC__ > 5
+			static const bool value = std::is_trivially_copyable<T>::value;
+		#else
+			// At least check it doesn't have virtual methods.
+			static const bool value = !std::is_polymorphic<T>::value;
+		#endif
+	};
 }
 
 namespace sw

src/Renderer/PixelProcessor.cpp

@@ -22,7 +22,7 @@
 #include "Shader/Constants.hpp"
 #include "Common/Debug.hpp"
 
-#include <string.h>
+#include <cstring>
 
 namespace sw
 {
@@ -32,12 +32,12 @@ namespace sw
 
 	bool precachePixel = false;
 
-	unsigned int PixelProcessor::States::computeHash()
+	uint32_t PixelProcessor::States::computeHash()
 	{
-		unsigned int *state = (unsigned int*)this;
-		unsigned int hash = 0;
+		uint32_t *state = reinterpret_cast<uint32_t*>(this);
+		uint32_t hash = 0;
 
-		for(unsigned int i = 0; i < sizeof(States) / 4; i++)
+		for(unsigned int i = 0; i < sizeof(States) / sizeof(uint32_t); i++)
 		{
 			hash ^= state[i];
 		}
@@ -45,11 +45,6 @@ namespace sw
 		return hash;
 	}
 
-	PixelProcessor::State::State()
-	{
-		memset(this, 0, sizeof(State));
-	}
-
 	bool PixelProcessor::State::operator==(const State &state) const
 	{
 		if(hash != state.hash)
@@ -57,6 +52,7 @@ namespace sw
 			return false;
 		}
 
+		static_assert(is_memcmparable<State>::value, "Cannot memcmp State");
 		return memcmp(static_cast<const States*>(this), static_cast<const States*>(&state), sizeof(States)) == 0;
 	}
 
@@ -79,7 +75,7 @@ namespace sw
 	PixelProcessor::~PixelProcessor()
 	{
 		delete routineCache;
-		routineCache = 0;
+		routineCache = nullptr;
 	}
 
 	void PixelProcessor::setFloatConstant(unsigned int index, const float value[4])

src/Renderer/PixelProcessor.hpp

@@ -28,9 +28,11 @@ namespace sw
 	class PixelProcessor
 	{
 	public:
-		struct States
+		struct States : Memset<States>
 		{
-			unsigned int computeHash();
+			States() : Memset(this, 0) {}
+
+			uint32_t computeHash();
 
 			int shaderID;
 
@@ -113,8 +115,6 @@ namespace sw
 
 		struct State : States
 		{
-			State();
-
 			bool operator==(const State &state) const;
 
 			int colorWriteActive(int index) const
@@ -132,7 +132,7 @@ namespace sw
 				return pixelFogMode != FOG_NONE;
 			}
 
-			unsigned int hash;
+			uint32_t hash;
 		};
 
 		struct Stencil

src/Renderer/SetupProcessor.cpp

@@ -22,6 +22,8 @@
 #include "Shader/Constants.hpp"
 #include "Common/Debug.hpp"
 
+#include <cstring>
+
 namespace sw
 {
 	extern bool complementaryDepthBuffer;
@@ -29,12 +31,12 @@ namespace sw
 
 	bool precacheSetup = false;
 
-	unsigned int SetupProcessor::States::computeHash()
+	uint32_t SetupProcessor::States::computeHash()
 	{
-		unsigned int *state = (unsigned int*)this;
-		unsigned int hash = 0;
+		uint32_t *state = reinterpret_cast<uint32_t*>(this);
+		uint32_t hash = 0;
 
-		for(unsigned int i = 0; i < sizeof(States) / 4; i++)
+		for(unsigned int i = 0; i < sizeof(States) / sizeof(uint32_t); i++)
 		{
 			hash ^= state[i];
 		}
@@ -42,11 +44,6 @@ namespace sw
 		return hash;
 	}
 
-	SetupProcessor::State::State(int i)
-	{
-		memset(this, 0, sizeof(State));
-	}
-
 	bool SetupProcessor::State::operator==(const State &state) const
 	{
 		if(hash != state.hash)
@@ -54,19 +51,20 @@ namespace sw
 			return false;
 		}
 
+		static_assert(is_memcmparable<State>::value, "Cannot memcmp States");
 		return memcmp(static_cast<const States*>(this), static_cast<const States*>(&state), sizeof(States)) == 0;
 	}
 
 	SetupProcessor::SetupProcessor(Context *context) : context(context)
 	{
-		routineCache = 0;
+		routineCache = nullptr;
 		setRoutineCacheSize(1024);
 	}
 
 	SetupProcessor::~SetupProcessor()
 	{
 		delete routineCache;
-		routineCache = 0;
+		routineCache = nullptr;
 	}
 
 	SetupProcessor::State SetupProcessor::update() const

src/Renderer/SetupProcessor.hpp

@@ -33,9 +33,11 @@ namespace sw
 	class SetupProcessor
 	{
 	public:
-		struct States
+		struct States : Memset<States>
 		{
-			unsigned int computeHash();
+			States() : Memset(this, 0) {}
+
+			uint32_t computeHash();
 
 			bool isDrawPoint               : 1;
 			bool isDrawLine                : 1;
@@ -76,11 +78,9 @@ namespace sw
 
 		struct State : States
 		{
-			State(int i = 0);
-
 			bool operator==(const State &states) const;
 
-			unsigned int hash;
+			uint32_t hash;
 		};
 
 		typedef bool (*RoutinePointer)(Primitive *primitive, const Triangle *triangle, const Polygon *polygon, const DrawData *draw);

src/Renderer/VertexProcessor.cpp

@@ -22,7 +22,7 @@
 #include "Common/Math.hpp"
 #include "Common/Debug.hpp"
 
-#include <string.h>
+#include <cstring>
 
 namespace sw
 {
@@ -36,12 +36,12 @@ namespace sw
 		}
 	}
 
-	unsigned int VertexProcessor::States::computeHash()
+	uint32_t VertexProcessor::States::computeHash()
 	{
-		unsigned int *state = (unsigned int*)this;
-		unsigned int hash = 0;
+		uint32_t *state = reinterpret_cast<uint32_t*>(this);
+		uint32_t hash = 0;
 
-		for(unsigned int i = 0; i < sizeof(States) / 4; i++)
+		for(unsigned int i = 0; i < sizeof(States) / sizeof(uint32_t); i++)
 		{
 			hash ^= state[i];
 		}
@@ -49,11 +49,6 @@ namespace sw
 		return hash;
 	}
 
-	VertexProcessor::State::State()
-	{
-		memset(this, 0, sizeof(State));
-	}
-
 	bool VertexProcessor::State::operator==(const State &state) const
 	{
 		if(hash != state.hash)
@@ -61,6 +56,7 @@ namespace sw
 			return false;
 		}
 
+		static_assert(is_memcmparable<State>::value, "Cannot memcmp States");
 		return memcmp(static_cast<const States*>(this), static_cast<const States*>(&state), sizeof(States)) == 0;
 	}
 
@@ -118,14 +114,14 @@ namespace sw
 			updateModelMatrix[i] = true;
 		}
 
-		routineCache = 0;
+		routineCache = nullptr;
 		setRoutineCacheSize(1024);
 	}
 
 	VertexProcessor::~VertexProcessor()
 	{
 		delete routineCache;
-		routineCache = 0;
+		routineCache = nullptr;
 	}
 
 	void VertexProcessor::setInputStream(int index, const Stream &stream)

src/Renderer/VertexProcessor.hpp

@@ -44,9 +44,11 @@ namespace sw
 	class VertexProcessor
 	{
 	public:
-		struct States
+		struct States : Memset<States>
 		{
-			unsigned int computeHash();
+			States() : Memset(this, 0) {}
+
+			uint32_t computeHash();
 
 			uint64_t shaderID;
 
@@ -140,11 +142,9 @@ namespace sw
 
 		struct State : States
 		{
-			State();
-
 			bool operator==(const State &state) const;
 
-			unsigned int hash;
+			uint32_t hash;
 		};
 
 		struct FixedFunction

src/Vulkan/VkDescriptorSetLayout.cpp

@@ -259,13 +259,13 @@ void SampledImageDescriptor::updateSampler(const vk::Sampler *newSampler)
 {
 	if(newSampler)
 	{
-		memcpy(&sampler, newSampler, sizeof(sampler));
+		memcpy(reinterpret_cast<void*>(&sampler), newSampler, sizeof(sampler));
 	}
 	else
 	{
 		// Descriptor ID's start at 1, allowing to detect descriptor update
 		// bugs by checking for 0. Also avoid reading random values.
-		memset(&sampler, 0, sizeof(sampler));
+		memset(reinterpret_cast<void*>(&sampler), 0, sizeof(sampler));
 	}
 }
 

src/WSI/VkSwapchainKHR.cpp

@@ -31,7 +31,7 @@ SwapchainKHR::SwapchainKHR(const VkSwapchainCreateInfoKHR *pCreateInfo, void *me
 	imageCount(pCreateInfo->minImageCount),
 	retired(false)
 {
-	memset(images, 0, imageCount * sizeof(PresentImage));
+	memset(reinterpret_cast<void*>(images), 0, imageCount * sizeof(PresentImage));
 }
 
 void SwapchainKHR::destroy(const VkAllocationCallbacks *pAllocator)

third_party/llvm-7.0/llvm/include/llvm/ADT/SmallVector.h

@@ -299,7 +299,7 @@ protected:
     // use memcpy here. Note that I and E are iterators and thus might be
     // invalid for memcpy if they are equal.
     if (I != E)
-      memcpy(Dest, I, (E - I) * sizeof(T));
+      memcpy(reinterpret_cast<void *>(Dest), I, (E - I) * sizeof(T));
   }
 
   /// Double the size of the allocated memory, guaranteeing space for at
@@ -310,7 +310,7 @@ public:
   void push_back(const T &Elt) {
     if (LLVM_UNLIKELY(this->size() >= this->capacity()))
       this->grow();
-    memcpy(this->end(), &Elt, sizeof(T));
+    memcpy(reinterpret_cast<void *>(this->end()), &Elt, sizeof(T));
     this->set_size(this->size() + 1);
   }