From b80ca8647e5b7c1d697be92ad371038d189b3e2d Mon Sep 17 00:00:00 2001 From: Hubert Tong Date: Sat, 30 Jul 2016 14:01:00 +0000 Subject: [PATCH] TrailingObjects::FixedSizeStorage constexpr fixes + tests Summary: This change fixes issues with `LLVM_CONSTEXPR` functions and `TrailingObjects::FixedSizeStorage`. In particular, some of the functions marked `LLVM_CONSTEXPR` used by `FixedSizeStorage` were not implemented such that they evaluate successfully as part of a constant expression despite constant arguments. This change also implements a more traditional template-meta path to accommodate MSVC, and adds unit tests for `FixedSizeStorage`. Drive-by fix: the access control for members of `TrailingObjectsImpl` is tightened. Reviewers: faisalv, rsmith, aaron.ballman Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D22668 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@277270 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/AlignOf.h | 2 +- include/llvm/Support/MathExtras.h | 21 ++++++++++ include/llvm/Support/TrailingObjects.h | 70 +++++++++++++++++++++++++------ unittests/Support/TrailingObjectsTest.cpp | 47 +++++++++++++++++++++ 4 files changed, 127 insertions(+), 13 deletions(-) diff --git a/include/llvm/Support/AlignOf.h b/include/llvm/Support/AlignOf.h index af7f20028b6..333bf160c10 100644 --- a/include/llvm/Support/AlignOf.h +++ b/include/llvm/Support/AlignOf.h @@ -103,7 +103,7 @@ template constexpr unsigned AlignOf::Alignment; /// class besides some cosmetic cleanliness. Example usage: /// alignOf() returns the alignment of an int. template -inline unsigned alignOf() { return AlignOf::Alignment; } +LLVM_CONSTEXPR inline unsigned alignOf() { return AlignOf::Alignment; } /// \struct AlignedCharArray /// \brief Helper for building an aligned character array type. diff --git a/include/llvm/Support/MathExtras.h b/include/llvm/Support/MathExtras.h index 704561c716e..fdf7f27381b 100644 --- a/include/llvm/Support/MathExtras.h +++ b/include/llvm/Support/MathExtras.h @@ -674,6 +674,27 @@ inline uint64_t alignTo(uint64_t Value, uint64_t Align, uint64_t Skew = 0) { return (Value + Align - 1 - Skew) / Align * Align + Skew; } +/// Returns the next integer (mod 2**64) that is greater than or equal to +/// \p Value and is a multiple of \c Align. \c Align must be non-zero. +template +LLVM_CONSTEXPR inline uint64_t alignTo(uint64_t Value) { + static_assert(Align != 0u, "Align must be non-zero"); + return (Value + Align - 1) / Align * Align; +} + +/// \c alignTo for contexts where a constant expression is required. +/// \sa alignTo +/// +/// \todo FIXME: remove when \c LLVM_CONSTEXPR becomes really \c constexpr +template +struct AlignTo { + static_assert(Align != 0u, "Align must be non-zero"); + template + struct from_value { + static const uint64_t value = (Value + Align - 1) / Align * Align; + }; +}; + /// Returns the largest uint64_t less than or equal to \p Value and is /// \p Skew mod \p Align. \p Align must be non-zero inline uint64_t alignDown(uint64_t Value, uint64_t Align, uint64_t Skew = 0) { diff --git a/include/llvm/Support/TrailingObjects.h b/include/llvm/Support/TrailingObjects.h index 5a21cddf973..427ae1cb3f6 100644 --- a/include/llvm/Support/TrailingObjects.h +++ b/include/llvm/Support/TrailingObjects.h @@ -127,29 +127,34 @@ template struct ExtractSecondType { template -struct TrailingObjectsImpl { +class TrailingObjectsImpl { // The main template definition is never used -- the two // specializations cover all possibilities. }; template -struct TrailingObjectsImpl +class TrailingObjectsImpl : public TrailingObjectsImpl { typedef TrailingObjectsImpl ParentType; - // Ensure the methods we inherit are not hidden. - using ParentType::getTrailingObjectsImpl; - using ParentType::additionalSizeToAllocImpl; + struct RequiresRealignment { + static const bool value = + llvm::AlignOf::Alignment < llvm::AlignOf::Alignment; + }; static LLVM_CONSTEXPR bool requiresRealignment() { - return llvm::AlignOf::Alignment < llvm::AlignOf::Alignment; + return RequiresRealignment::value; } +protected: + // Ensure the inherited getTrailingObjectsImpl is not hidden. + using ParentType::getTrailingObjectsImpl; + // These two functions are helper functions for // TrailingObjects::getTrailingObjects. They recurse to the left -- // the result for each type in the list of trailing types depends on @@ -195,20 +200,37 @@ struct TrailingObjectsImpl::type... MoreCounts) { - return additionalSizeToAllocImpl( + return ParentType::additionalSizeToAllocImpl( (requiresRealignment() - ? llvm::alignTo(SizeSoFar, llvm::alignOf()) + ? llvm::alignTo::Alignment>(SizeSoFar) : SizeSoFar) + sizeof(NextTy) * Count1, MoreCounts...); } + + // additionalSizeToAllocImpl for contexts where a constant expression is + // required. + // FIXME: remove when LLVM_CONSTEXPR becomes really constexpr + template + struct AdditionalSizeToAllocImpl { + static_assert(sizeof...(MoreTys) == sizeof...(MoreCounts), + "Number of counts do not match number of types"); + static const size_t value = ParentType::template AdditionalSizeToAllocImpl< + (RequiresRealignment::value + ? llvm::AlignTo::Alignment>:: + template from_value::value + : SizeSoFar) + + sizeof(NextTy) * Count1, + MoreCounts...>::value; + }; }; // The base case of the TrailingObjectsImpl inheritance recursion, // when there's no more trailing types. template -struct TrailingObjectsImpl +class TrailingObjectsImpl : public TrailingObjectsAligner { +protected: // This is a dummy method, only here so the "using" doesn't fail -- // it will never be called, because this function recurses backwards // up the inheritance chain to subclasses. @@ -218,6 +240,13 @@ struct TrailingObjectsImpl return SizeSoFar; } + // additionalSizeToAllocImpl for contexts where a constant expression is + // required. + // FIXME: remove when LLVM_CONSTEXPR becomes really constexpr + template struct AdditionalSizeToAllocImpl { + static const size_t value = SizeSoFar; + }; + template static void verifyTrailingObjectsAlignment() {} }; @@ -235,7 +264,7 @@ class TrailingObjects : private trailing_objects_internal::TrailingObjectsImpl< BaseTy, TrailingTys...> { template - friend struct trailing_objects_internal::TrailingObjectsImpl; + friend class trailing_objects_internal::TrailingObjectsImpl; template class Foo {}; @@ -343,6 +372,21 @@ public: return sizeof(BaseTy) + ParentType::additionalSizeToAllocImpl(0, Counts...); } + // totalSizeToAlloc for contexts where a constant expression is required. + // FIXME: remove when LLVM_CONSTEXPR becomes really constexpr + template struct TotalSizeToAlloc { + static_assert( + std::is_same, Foo>::value, + "Arguments to TotalSizeToAlloc do not match with TrailingObjects"); + template struct with_counts { + static_assert(sizeof...(TrailingTys) == sizeof...(Counts), + "Number of counts do not match number of types"); + static const size_t value = + sizeof(BaseTy) + + ParentType::template AdditionalSizeToAllocImpl<0, Counts...>::value; + }; + }; + /// A type where its ::with_counts template member has a ::type member /// suitable for use as uninitialized storage for an object with the given /// trailing object counts. The template arguments are similar to those @@ -360,7 +404,9 @@ public: /// \endcode template struct FixedSizeStorage { template struct with_counts { - enum { Size = totalSizeToAlloc(Counts...) }; + enum { + Size = TotalSizeToAlloc::template with_counts::value + }; typedef llvm::AlignedCharArray< llvm::AlignOf::Alignment, Size > type; diff --git a/unittests/Support/TrailingObjectsTest.cpp b/unittests/Support/TrailingObjectsTest.cpp index a1d3e7b3c86..65fdcb93623 100644 --- a/unittests/Support/TrailingObjectsTest.cpp +++ b/unittests/Support/TrailingObjectsTest.cpp @@ -41,6 +41,9 @@ public: unsigned numShorts() const { return NumShorts; } // Pull some protected members in as public, for testability. + template + using FixedSizeStorage = TrailingObjects::FixedSizeStorage; + using TrailingObjects::totalSizeToAlloc; using TrailingObjects::additionalSizeToAlloc; using TrailingObjects::getTrailingObjects; @@ -94,6 +97,9 @@ public: } // Pull some protected members in as public, for testability. + template + using FixedSizeStorage = TrailingObjects::FixedSizeStorage; + using TrailingObjects::totalSizeToAlloc; using TrailingObjects::additionalSizeToAlloc; using TrailingObjects::getTrailingObjects; @@ -106,7 +112,20 @@ TEST(TrailingObjects, OneArg) { EXPECT_EQ(Class1::additionalSizeToAlloc(1), sizeof(short)); EXPECT_EQ(Class1::additionalSizeToAlloc(3), sizeof(short) * 3); + EXPECT_EQ( + llvm::alignOf(), + llvm::alignOf::with_counts<1>::type>()); + EXPECT_EQ(sizeof(Class1::FixedSizeStorage::with_counts<1>::type), + llvm::alignTo(Class1::totalSizeToAlloc(1), + llvm::alignOf())); EXPECT_EQ(Class1::totalSizeToAlloc(1), sizeof(Class1) + sizeof(short)); + + EXPECT_EQ( + llvm::alignOf(), + llvm::alignOf::with_counts<3>::type>()); + EXPECT_EQ(sizeof(Class1::FixedSizeStorage::with_counts<3>::type), + llvm::alignTo(Class1::totalSizeToAlloc(3), + llvm::alignOf())); EXPECT_EQ(Class1::totalSizeToAlloc(3), sizeof(Class1) + sizeof(short) * 3); @@ -131,6 +150,14 @@ TEST(TrailingObjects, TwoArg) { EXPECT_EQ((Class2::additionalSizeToAlloc(3, 1)), sizeof(double) * 3 + sizeof(short)); + EXPECT_EQ( + llvm::alignOf(), + (llvm::alignOf< + Class2::FixedSizeStorage::with_counts<1, 1>::type>())); + EXPECT_EQ( + sizeof(Class2::FixedSizeStorage::with_counts<1, 1>::type), + llvm::alignTo(Class2::totalSizeToAlloc(1, 1), + llvm::alignOf())); EXPECT_EQ((Class2::totalSizeToAlloc(1, 1)), sizeof(Class2) + sizeof(double) + sizeof(short)); @@ -165,6 +192,16 @@ TEST(TrailingObjects, ThreeArg) { EXPECT_EQ((Class3::additionalSizeToAlloc(1, 1, 3)), sizeof(double) + sizeof(short) + 3 * sizeof(bool)); EXPECT_EQ(sizeof(Class3), llvm::alignTo(1, llvm::alignOf())); + + EXPECT_EQ(llvm::alignOf(), + (llvm::alignOf::with_counts<1, 1, 3>::type>())); + EXPECT_EQ( + sizeof(Class3::FixedSizeStorage::with_counts<1, 1, 3>::type), + llvm::alignTo(Class3::totalSizeToAlloc(1, 1, 3), + llvm::alignOf())); + std::unique_ptr P(new char[1000]); Class3 *C = reinterpret_cast(P.get()); EXPECT_EQ(C->getTrailingObjects(), reinterpret_cast(C + 1)); @@ -186,6 +223,16 @@ TEST(TrailingObjects, Realignment) { EXPECT_EQ((Class4::additionalSizeToAlloc(1, 1)), llvm::alignTo(sizeof(long) + 1, llvm::alignOf())); EXPECT_EQ(sizeof(Class4), llvm::alignTo(1, llvm::alignOf())); + + EXPECT_EQ( + llvm::alignOf(), + (llvm::alignOf< + Class4::FixedSizeStorage::with_counts<1, 1>::type>())); + EXPECT_EQ( + sizeof(Class4::FixedSizeStorage::with_counts<1, 1>::type), + llvm::alignTo(Class4::totalSizeToAlloc(1, 1), + llvm::alignOf())); + std::unique_ptr P(new char[1000]); Class4 *C = reinterpret_cast(P.get()); EXPECT_EQ(C->getTrailingObjects(), reinterpret_cast(C + 1)); -- 2.11.0