From 43b57b49bf330278c252248ab59c6ab31fa3fa33 Mon Sep 17 00:00:00 2001 From: Kazu Hirata Date: Tue, 23 Sep 2025 16:28:24 -0700 Subject: [PATCH] [Support] Clean up Align This patch cleans up the constructors and operator= of Align. - "constexpr Align(LogValue CA)" is a really strange "public:" constructor. It can only be constructed with a private struct LogValue, which wraps the log2 of an alignment. Since nobody uses it outside the class anyway, this patch moves the constructor to "private:" while switching to a tag-based parameter list. In turn, this patch removes LogValue. - The block of comment being deleted is no longer applicable, so this patch marks operator= constexpr. To test operator= in a unit test, this patch makes value() constexpr also. Refer to the unit test to see how operator= and value() are put together. --- llvm/include/llvm/Support/Alignment.h | 23 ++++++----------------- llvm/unittests/Support/AlignmentTest.cpp | 10 ++++++++++ 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/llvm/include/llvm/Support/Alignment.h b/llvm/include/llvm/Support/Alignment.h index 84773f1d9c37b..a4ca54e26f18d 100644 --- a/llvm/include/llvm/Support/Alignment.h +++ b/llvm/include/llvm/Support/Alignment.h @@ -52,16 +52,8 @@ struct Align { friend unsigned encode(struct MaybeAlign A); friend struct MaybeAlign decodeMaybeAlign(unsigned Value); - /// A trivial type to allow construction of constexpr Align. - /// This is currently needed to workaround a bug in GCC 5.3 which prevents - /// definition of constexpr assign operators. - /// https://stackoverflow.com/questions/46756288/explicitly-defaulted-function-cannot-be-declared-as-constexpr-because-the-implic - /// FIXME: Remove this, make all assign operators constexpr and introduce user - /// defined literals when we don't have to support GCC 5.3 anymore. - /// https://llvm.org/docs/GettingStarted.html#getting-a-modern-host-c-toolchain - struct LogValue { - uint8_t Log; - }; + struct FromShiftValue {}; + constexpr Align(FromShiftValue, uint8_t Shift) : ShiftValue(Shift) {} public: /// Default is byte-aligned. @@ -70,8 +62,8 @@ struct Align { /// checks have been performed when building `Other`. constexpr Align(const Align &Other) = default; constexpr Align(Align &&Other) = default; - Align &operator=(const Align &Other) = default; - Align &operator=(Align &&Other) = default; + constexpr Align &operator=(const Align &Other) = default; + constexpr Align &operator=(Align &&Other) = default; explicit Align(uint64_t Value) { assert(Value > 0 && "Value must not be 0"); @@ -82,7 +74,7 @@ struct Align { /// This is a hole in the type system and should not be abused. /// Needed to interact with C for instance. - uint64_t value() const { return uint64_t(1) << ShiftValue; } + constexpr uint64_t value() const { return uint64_t(1) << ShiftValue; } // Returns the previous alignment. Align previous() const { @@ -94,7 +86,7 @@ struct Align { /// Allow constructions of constexpr Align. template constexpr static Align Constant() { - return LogValue{static_cast(ConstantLog2())}; + return Align(FromShiftValue{}, ConstantLog2()); } /// Allow constructions of constexpr Align from types. @@ -102,9 +94,6 @@ struct Align { template constexpr static Align Of() { return Constant>(); } - - /// Constexpr constructor from LogValue type. - constexpr Align(LogValue CA) : ShiftValue(CA.Log) {} }; /// Treats the value 0 as a 1, so Align is always at least 1. diff --git a/llvm/unittests/Support/AlignmentTest.cpp b/llvm/unittests/Support/AlignmentTest.cpp index 7b771977027b4..3a4416128d0e5 100644 --- a/llvm/unittests/Support/AlignmentTest.cpp +++ b/llvm/unittests/Support/AlignmentTest.cpp @@ -44,6 +44,16 @@ TEST(AlignmentTest, AlignConstexprConstant) { EXPECT_EQ(Align(alignof(uint64_t)), kConstantAlign); } +TEST(AlignmentTest, ConstexprAssign) { + constexpr auto assignAndGet = []() constexpr { + Align A = Align::Constant<8>(); + Align B = Align::Constant<16>(); + A = B; + return A.value(); + }; + static_assert(assignAndGet() == 16); +} + std::vector getValidAlignments() { std::vector Out; for (size_t Shift = 0; Shift < 64; ++Shift)