From 88b3673760d8e62bae608b832226caa5fd3ac20f Mon Sep 17 00:00:00 2001 From: Bryan Bernhart Date: Fri, 26 Aug 2022 12:27:10 -0700 Subject: [PATCH] Remove enum flags dependency. Replaces custom enum flag type with Windows-defined DEFINE_ENUM_FLAG_OPERATORS. This avoids the need to export new types and allows the platform to conditionally provide one, if not defined. Support for a minimum viable implementation #577 --- src/gpgmm/d3d12/EventRecordD3D12.h | 11 +- src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp | 4 +- src/gpgmm/d3d12/ResourceAllocatorD3D12.h | 15 +- src/gpgmm/utils/BUILD.gn | 2 +- src/gpgmm/utils/CMakeLists.txt | 2 +- src/gpgmm/utils/EnumFlags.h | 93 ++++++++++ src/gpgmm/utils/Flags.h | 165 ------------------ src/gpgmm/vk/ResourceAllocatorVk.h | 1 - src/tests/BUILD.gn | 2 +- src/tests/CMakeLists.txt | 2 +- .../D3D12EventTraceReplay.cpp | 4 +- .../{FlagsTests.cpp => EnumFlagsTests.cpp} | 39 ++--- 12 files changed, 130 insertions(+), 210 deletions(-) create mode 100644 src/gpgmm/utils/EnumFlags.h delete mode 100644 src/gpgmm/utils/Flags.h rename src/tests/unittests/{FlagsTests.cpp => EnumFlagsTests.cpp} (68%) diff --git a/src/gpgmm/d3d12/EventRecordD3D12.h b/src/gpgmm/d3d12/EventRecordD3D12.h index 03334e048..a61a32602 100644 --- a/src/gpgmm/d3d12/EventRecordD3D12.h +++ b/src/gpgmm/d3d12/EventRecordD3D12.h @@ -16,7 +16,9 @@ #define GPGMM_D3D12_EVENTRECORDD3D12_H_ #include "gpgmm/d3d12/d3d12_platform.h" -#include "gpgmm/utils/Flags.h" +#include "gpgmm/utils/EnumFlags.h" + +#include namespace gpgmm::d3d12 { @@ -57,8 +59,7 @@ namespace gpgmm::d3d12 { EVENT_RECORD_FLAG_ALL_EVENTS = 0xFF, }; - using EVENT_RECORD_FLAGS_TYPE = Flags; - DEFINE_OPERATORS_FOR_FLAGS(EVENT_RECORD_FLAGS_TYPE) + DEFINE_ENUM_FLAG_OPERATORS(EVENT_RECORD_FLAGS) /** \enum EVENT_RECORD_SCOPE Represents recording scopes to limit event recording. @@ -82,7 +83,7 @@ namespace gpgmm::d3d12 { Optional parameter. By default, nothing is recorded. */ - EVENT_RECORD_FLAGS_TYPE Flags = EVENT_RECORD_FLAG_NONE; + EVENT_RECORD_FLAGS Flags = EVENT_RECORD_FLAG_NONE; /** \brief Minimum severity level to record messages. @@ -113,4 +114,4 @@ namespace gpgmm::d3d12 { } // namespace gpgmm::d3d12 -#endif // GPGMM_D3D12_EVENTRECORDD3D12_H_ \ No newline at end of file +#endif // GPGMM_D3D12_EVENTRECORDD3D12_H_ diff --git a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp index 964b81b66..5d29f1c8a 100644 --- a/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp +++ b/src/gpgmm/d3d12/ResourceAllocatorD3D12.cpp @@ -146,7 +146,7 @@ namespace gpgmm::d3d12 { uint64_t GetHeapAlignment(D3D12_HEAP_FLAGS heapFlags, bool allowMSAA) { const D3D12_HEAP_FLAGS denyAllTexturesFlags = D3D12_HEAP_FLAG_DENY_RT_DS_TEXTURES | D3D12_HEAP_FLAG_DENY_NON_RT_DS_TEXTURES; - if (Flags(heapFlags).HasFlags(denyAllTexturesFlags)) { + if (HasAllFlags(heapFlags, denyAllTexturesFlags)) { return D3D12_DEFAULT_RESOURCE_PLACEMENT_ALIGNMENT; } @@ -758,7 +758,7 @@ namespace gpgmm::d3d12 { // Check memory requirements. bool isAlwaysCommitted = mIsAlwaysCommitted; D3D12_HEAP_FLAGS heapFlags = GetHeapFlags(resourceHeapType, IsCreateHeapNotResident()); - if (!Flags(heapFlags).HasFlags(allocationDescriptor.ExtraRequiredHeapFlags)) { + if (!HasAllFlags(heapFlags, allocationDescriptor.ExtraRequiredHeapFlags)) { DebugEvent(GetTypename()) << "Required heap flags are incompatible with resource heap type (" << std::to_string(allocationDescriptor.ExtraRequiredHeapFlags) << " vs " diff --git a/src/gpgmm/d3d12/ResourceAllocatorD3D12.h b/src/gpgmm/d3d12/ResourceAllocatorD3D12.h index 216897841..37f15e3d7 100644 --- a/src/gpgmm/d3d12/ResourceAllocatorD3D12.h +++ b/src/gpgmm/d3d12/ResourceAllocatorD3D12.h @@ -19,7 +19,7 @@ #include "gpgmm/common/MemoryAllocator.h" #include "gpgmm/d3d12/EventRecordD3D12.h" #include "gpgmm/d3d12/IUnknownImplD3D12.h" -#include "gpgmm/utils/Flags.h" +#include "gpgmm/utils/EnumFlags.h" #include "include/gpgmm_export.h" #include @@ -80,8 +80,7 @@ namespace gpgmm::d3d12 { ALLOCATOR_FLAG_ALWAYS_ON_DEMAND = 0x8, }; - using ALLOCATOR_FLAGS_TYPE = Flags; - DEFINE_OPERATORS_FOR_FLAGS(ALLOCATOR_FLAGS_TYPE) + DEFINE_ENUM_FLAG_OPERATORS(ALLOCATOR_FLAGS) /** \enum ALLOCATOR_ALGORITHM Specify the algorithms used for allocation. @@ -127,8 +126,7 @@ namespace gpgmm::d3d12 { ALLOCATOR_ALGORITHM_SEGMENTED_POOL = 0x3, }; - using ALLOCATOR_ALGORITHM_TYPE = Flags; - DEFINE_OPERATORS_FOR_FLAGS(ALLOCATOR_ALGORITHM_TYPE) + DEFINE_ENUM_FLAG_OPERATORS(ALLOCATOR_ALGORITHM) /** \struct ALLOCATOR_DESC Specify parameters for creating allocators. @@ -151,7 +149,7 @@ namespace gpgmm::d3d12 { For example, whether the allocator can reuse memory, or resources should be resident upon creation. */ - ALLOCATOR_FLAGS_TYPE Flags = ALLOCATOR_FLAG_NONE; + ALLOCATOR_FLAGS Flags = ALLOCATOR_FLAG_NONE; /** \brief Minimum severity level to log messages to console. @@ -303,8 +301,7 @@ namespace gpgmm::d3d12 { ALLOCATION_FLAG_ALWAYS_CACHE_SIZE = 0x10, }; - using ALLOCATION_FLAGS_TYPE = Flags; - DEFINE_OPERATORS_FOR_FLAGS(ALLOCATION_FLAGS_TYPE) + DEFINE_ENUM_FLAG_OPERATORS(ALLOCATION_FLAGS) /** \struct ALLOCATION_FLAGS Specifies how allocations should be created. @@ -314,7 +311,7 @@ namespace gpgmm::d3d12 { Optional parameter. By default, GPGMM will decide automatically. */ - ALLOCATION_FLAGS_TYPE Flags = ALLOCATION_FLAG_NONE; + ALLOCATION_FLAGS Flags = ALLOCATION_FLAG_NONE; /** \brief Heap type that the resource to be allocated requires. diff --git a/src/gpgmm/utils/BUILD.gn b/src/gpgmm/utils/BUILD.gn index 50ce70239..89513aa02 100644 --- a/src/gpgmm/utils/BUILD.gn +++ b/src/gpgmm/utils/BUILD.gn @@ -25,7 +25,7 @@ if (is_win || is_linux || is_chromeos || is_mac || is_fuchsia || is_android) { "Assert.cpp", "Assert.h", "Compiler.h", - "Flags.h", + "EnumFlags.h", "JSONEncoder.cpp", "JSONEncoder.h", "Limits.h", diff --git a/src/gpgmm/utils/CMakeLists.txt b/src/gpgmm/utils/CMakeLists.txt index 8c7fcc838..7c10bed83 100644 --- a/src/gpgmm/utils/CMakeLists.txt +++ b/src/gpgmm/utils/CMakeLists.txt @@ -17,7 +17,7 @@ target_sources(gpgmm_utils PRIVATE "Assert.cpp" "Assert.h" "Compiler.h" - "Flags.h" + "EnumFlags.h" "JSONEncoder.cpp" "JSONEncoder.h" "Limits.h" diff --git a/src/gpgmm/utils/EnumFlags.h b/src/gpgmm/utils/EnumFlags.h new file mode 100644 index 000000000..34ca2f11c --- /dev/null +++ b/src/gpgmm/utils/EnumFlags.h @@ -0,0 +1,93 @@ +// Copyright 2021 The GPGMM Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef GPGMM_UTILS_ENUMFLAGS_H_ +#define GPGMM_UTILS_ENUMFLAGS_H_ + +#include + +namespace gpgmm { + + // Returns true if all flags of B are in A. + template + bool HasAllFlags(const EnumFlagsType1& a, const EnumFlagsType2& b) { + return (a & b) == b; + } + +// Define enum-operator overloads to enable bit-wise operations on enum types that are used to +// define flags. Use DEFINE_ENUM_FLAG_OPERATORS(MyType) to enable these operators for MyType. +#ifndef DEFINE_ENUM_FLAG_OPERATORS + + template + struct EnumFlagTrait; + + template <> + struct EnumFlagTrait<1> { + typedef int8_t SizeType; + }; + + template <> + struct EnumFlagTrait<2> { + typedef int16_t SizeType; + }; + + template <> + struct EnumFlagTrait<4> { + typedef int32_t SizeType; + }; + + template + struct EnumFlagInteger { + typedef typename EnumFlagTrait::SizeType SizeType; + }; + +// Overloaded operations are inline to avoid duplicate symbols from being +// generated and failing to link when using the macro across multiple cpps. +# define DEFINE_ENUM_FLAG_OPERATORS(EnumType) \ + inline EnumType operator|(EnumType a, EnumType b) { \ + return EnumType((static_cast::SizeType>(a)) | \ + static_cast::SizeType>(b)); \ + } \ + inline EnumType& operator|=(EnumType& a, EnumType b) { \ + return reinterpret_cast( \ + (reinterpret_cast::SizeType&>(a)) |= \ + (static_cast::SizeType>(b))); \ + } \ + inline EnumType operator&(EnumType a, EnumType b) { \ + return EnumType((static_cast::SizeType>(a)) & \ + (static_cast::SizeType>(b))); \ + } \ + inline EnumType& operator&=(EnumType& a, EnumType b) { \ + return reinterpret_cast( \ + (reinterpret_cast::SizeType&>(a)) &= \ + (static_cast::SizeType>(b))); \ + } \ + inline EnumType operator~(EnumType a) { \ + return EnumType(~(static_cast::SizeType>(a))); \ + } \ + inline EnumType operator^(EnumType a, EnumType b) { \ + return EnumType((static_cast::SizeType>(a)) ^ \ + (static_cast::SizeType>(b))); \ + } \ + inline EnumType& operator^=(EnumType& a, EnumType b) { \ + return reinterpret_cast( \ + (reinterpret_cast::SizeType&>(a)) ^= \ + (static_cast::SizeType>(b))); \ + } + +#endif // DEFINE_ENUM_FLAG_OPERATORS + +} // namespace gpgmm + +#endif // GPGMM_UTILS_ENUMFLAGS_H_ diff --git a/src/gpgmm/utils/Flags.h b/src/gpgmm/utils/Flags.h deleted file mode 100644 index 41af4791d..000000000 --- a/src/gpgmm/utils/Flags.h +++ /dev/null @@ -1,165 +0,0 @@ -// Copyright 2021 The GPGMM Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef GPGMM_UTILS_FLAGS_H_ -#define GPGMM_UTILS_FLAGS_H_ - -#include - -namespace gpgmm { - - // Flags is a type-safe means of storing enum values that can be OR'd - // together without requiring a cast. - template - class Flags final { - public: - using enum_type = EnumT; - using value_type = ValueT; - - constexpr Flags() : mValue(0) { - } - - constexpr Flags(EnumT flag) : mValue(static_cast(flag)) { - } - - constexpr explicit Flags(ValueT value) : mValue(static_cast(value)) { - } - - constexpr bool operator==(EnumT flag) const { - return mValue == static_cast(flag); - } - - constexpr bool operator!=(EnumT flag) const { - return mValue != static_cast(flag); - } - - constexpr friend std::ostream& operator<<(std::ostream& out, const Flags& flag) { - out << flag.mValue; - return out; - } - - // Compound assignment between Flags. - - Flags& operator&=(const Flags& flags) { - mValue &= flags.mValue; - return *this; - } - - Flags& operator|=(const Flags& flags) { - mValue |= flags.mValue; - return *this; - } - - Flags& operator^=(const Flags& flags) { - mValue ^= flags.mValue; - return *this; - } - - // Bitwise between Flags. - - constexpr Flags operator&(const Flags& flags) const { - return Flags(mValue & flags.mValue); - } - - constexpr Flags operator|(const Flags& flags) const { - return Flags(mValue | flags.mValue); - } - - constexpr Flags operator^(const Flags& flags) const { - return Flags(mValue ^ flags.mValue); - } - - // Bitwise using raw enum. - - constexpr Flags operator&(EnumT flag) const { - return operator&(Flags(flag)); - } - - constexpr Flags operator|(EnumT flag) const { - return operator|(Flags(flag)); - } - - constexpr Flags operator^(EnumT flag) const { - return operator^(Flags(flag)); - } - - // Compound assignment using raw enum. - - Flags& operator&=(EnumT flag) { - return operator&=(Flags(flag)); - } - - Flags& operator|=(EnumT flag) { - return operator|=(Flags(flag)); - } - - Flags& operator^=(EnumT flag) { - return operator^=(Flags(flag)); - } - - constexpr Flags operator~() const { - return Flags(~mValue); - } - - constexpr bool operator!() const { - return !mValue; - } - - operator int() const { - return mValue; - } - - // Returns true if |this| has all |flags|. - bool HasFlags(const Flags& flags) const { - return (mValue & flags) == flags; - } - - private: - ValueT mValue; - }; - -// Overloaded operations are inline to avoid duplicate symbols from being -// generated and failing to link when using the macro across multiple cpps. -#define DEFINE_OPERATORS_FOR_FLAGS(Type) \ - inline constexpr Type operator&(Type::enum_type lhs, Type::enum_type rhs) { \ - return Type(lhs) & rhs; \ - } \ - inline constexpr Type operator&(Type::enum_type lhs, const Type& rhs) { \ - return rhs & lhs; \ - } \ - inline void operator&(Type::enum_type lhs, Type::value_type rhs) { \ - } \ - inline constexpr Type operator|(Type::enum_type lhs, Type::enum_type rhs) { \ - return Type(lhs) | rhs; \ - } \ - inline constexpr Type operator|(Type::enum_type lhs, const Type& rhs) { \ - return rhs | lhs; \ - } \ - inline void operator|(Type::enum_type lhs, Type::value_type rhs) { \ - } \ - inline constexpr Type operator^(Type::enum_type lhs, Type::enum_type rhs) { \ - return Type(lhs) ^ rhs; \ - } \ - inline constexpr Type operator^(Type::enum_type lhs, const Type& rhs) { \ - return rhs ^ lhs; \ - } \ - inline void operator^(Type::enum_type lhs, Type::value_type rhs) { \ - } \ - inline constexpr Type operator~(Type::enum_type val) { \ - return ~Type(val); \ - } - -} // namespace gpgmm - -#endif // GPGMM_UTILS_FLAGS_H_ diff --git a/src/gpgmm/vk/ResourceAllocatorVk.h b/src/gpgmm/vk/ResourceAllocatorVk.h index 3ab9d403d..f202f1fa5 100644 --- a/src/gpgmm/vk/ResourceAllocatorVk.h +++ b/src/gpgmm/vk/ResourceAllocatorVk.h @@ -16,7 +16,6 @@ #define GPGMM_VK_RESOURCEALLOCATORVK_H_ #include "gpgmm/common/MemoryAllocator.h" -#include "gpgmm/utils/Flags.h" #include "gpgmm/vk/FunctionsVk.h" #include "include/gpgmm_export.h" diff --git a/src/tests/BUILD.gn b/src/tests/BUILD.gn index 1273f59ae..da5d67a68 100644 --- a/src/tests/BUILD.gn +++ b/src/tests/BUILD.gn @@ -107,8 +107,8 @@ test("gpgmm_unittests") { "unittests/BuddyBlockAllocatorTests.cpp", "unittests/BuddyMemoryAllocatorTests.cpp", "unittests/ConditionalMemoryAllocatorTests.cpp", + "unittests/EnumFlagsTests.cpp", "unittests/EventTraceWriterTests.cpp", - "unittests/FlagsTests.cpp", "unittests/LinkedListTests.cpp", "unittests/MathTests.cpp", "unittests/MemoryAllocatorTests.cpp", diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 2dae48690..d38e649cf 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -26,8 +26,8 @@ target_sources(gpgmm_unittests PRIVATE "DummyMemoryAllocator.h" "unittests/BuddyBlockAllocatorTests.cpp" "unittests/ConditionalMemoryAllocatorTests.cpp" + "unittests/EnumFlagsTests.cpp" "unittests/EventTraceWriterTests.cpp" - "unittests/FlagsTests.cpp" "unittests/LinkedListTests.cpp" "unittests/MathTests.cpp" "unittests/MemoryAllocatorTests.cpp" diff --git a/src/tests/capture_replay_tests/D3D12EventTraceReplay.cpp b/src/tests/capture_replay_tests/D3D12EventTraceReplay.cpp index 389acaeb5..41138a143 100644 --- a/src/tests/capture_replay_tests/D3D12EventTraceReplay.cpp +++ b/src/tests/capture_replay_tests/D3D12EventTraceReplay.cpp @@ -319,7 +319,7 @@ class D3D12EventTraceReplay : public D3D12TestBase, public CaptureReplayTestWith if (envParams.CaptureEventMask != 0) { residencyDesc.RecordOptions.Flags |= - static_cast(envParams.CaptureEventMask); + static_cast(envParams.CaptureEventMask); residencyDesc.RecordOptions.TraceFile = traceFile.path; residencyDesc.RecordOptions.MinMessageLevel = GetMessageSeverity(GetLogLevel()); @@ -401,7 +401,7 @@ class D3D12EventTraceReplay : public D3D12TestBase, public CaptureReplayTestWith if (envParams.CaptureEventMask != 0) { allocatorDesc.RecordOptions.Flags |= - static_cast(envParams.CaptureEventMask); + static_cast(envParams.CaptureEventMask); allocatorDesc.RecordOptions.TraceFile = traceFile.path; allocatorDesc.RecordOptions.MinMessageLevel = GetMessageSeverity(GetLogLevel()); diff --git a/src/tests/unittests/FlagsTests.cpp b/src/tests/unittests/EnumFlagsTests.cpp similarity index 68% rename from src/tests/unittests/FlagsTests.cpp rename to src/tests/unittests/EnumFlagsTests.cpp index 8b187021a..325894baf 100644 --- a/src/tests/unittests/FlagsTests.cpp +++ b/src/tests/unittests/EnumFlagsTests.cpp @@ -14,27 +14,26 @@ #include -#include "gpgmm/utils/Flags.h" +// Make sure DEFINE_ENUM_FLAG_OPERATORS is being tested is from GPGMM. +#undef DEFINE_ENUM_FLAG_OPERATORS + +#include "gpgmm/utils/EnumFlags.h" #include namespace gpgmm { - enum Count { + enum CountFlags { kZero, kOne = 1, kTwo = 2, kThree = 3, }; - using CountFlags = Flags; - DEFINE_OPERATORS_FOR_FLAGS(CountFlags) + DEFINE_ENUM_FLAG_OPERATORS(CountFlags) - TEST(FlagsTests, Basic) { - CountFlags a; - a |= kZero; - EXPECT_TRUE(a == kZero); - EXPECT_EQ(a, 0); + TEST(EnumFlagsTests, Basic) { + CountFlags a = kZero; a |= kOne; EXPECT_TRUE(a == kOne); @@ -54,21 +53,19 @@ namespace gpgmm { } struct Desc { - enum Option { + enum OptionFlags { kNone, kFirst = 1, kSecond = 2, kThird = 3, kFourth = 4, }; - - using OptionFlags = Flags