From 59f29d8aaea51f42d7c41c387fb099f224afb0cd Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Wed, 3 Jul 2024 11:39:57 +0100 Subject: [PATCH 1/4] DynamicAPInt: optimize size of structure Reuse the APInt::BitWidth to eliminate DynamicAPInt::HoldsLarge, cutting the size of DynamicAPInt by four bytes. This is implemented by making DynamicAPInt a friend of SlowDynamicAPInt and APInt, so it can directly access SlowDynamicAPInt::Val and APInt::BitWidth. --- llvm/include/llvm/ADT/APInt.h | 4 ++++ llvm/include/llvm/ADT/DynamicAPInt.h | 23 ++++++++++++----------- llvm/include/llvm/ADT/SlowDynamicAPInt.h | 7 +++++++ 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/llvm/include/llvm/ADT/APInt.h b/llvm/include/llvm/ADT/APInt.h index 6cfa6ec665084..108df7e0eaeaa 100644 --- a/llvm/include/llvm/ADT/APInt.h +++ b/llvm/include/llvm/ADT/APInt.h @@ -30,6 +30,7 @@ class StringRef; class hash_code; class raw_ostream; struct Align; +class DynamicAPInt; template class SmallVectorImpl; template class ArrayRef; @@ -1895,6 +1896,9 @@ class [[nodiscard]] APInt { friend struct DenseMapInfo; friend class APSInt; + // Make DynamicAPInt a friend so it can access BitWidth directly. + friend DynamicAPInt; + /// This constructor is used only internally for speed of construction of /// temporaries. It is unsafe since it takes ownership of the pointer, so it /// is not public. diff --git a/llvm/include/llvm/ADT/DynamicAPInt.h b/llvm/include/llvm/ADT/DynamicAPInt.h index f312f776df971..03325d3313b5a 100644 --- a/llvm/include/llvm/ADT/DynamicAPInt.h +++ b/llvm/include/llvm/ADT/DynamicAPInt.h @@ -35,21 +35,20 @@ namespace llvm { /// We always_inline all operations; removing these results in a 1.5x /// performance slowdown. /// -/// When HoldsLarge is true, a SlowMPInt is held in the union. If it is false, -/// the int64_t is held. Using std::variant instead would lead to significantly -/// worse performance. +/// When isLarge returns true, a SlowMPInt is held in the union. If isSmall +/// returns true, the int64_t is held. Using std::variant instead would lead to +/// significantly worse performance. class DynamicAPInt { union { int64_t ValSmall; detail::SlowDynamicAPInt ValLarge; }; - unsigned HoldsLarge; LLVM_ATTRIBUTE_ALWAYS_INLINE void initSmall(int64_t O) { if (LLVM_UNLIKELY(isLarge())) ValLarge.detail::SlowDynamicAPInt::~SlowDynamicAPInt(); ValSmall = O; - HoldsLarge = false; + ValLarge.Val.BitWidth = 0; } LLVM_ATTRIBUTE_ALWAYS_INLINE void initLarge(const detail::SlowDynamicAPInt &O) { @@ -66,14 +65,13 @@ class DynamicAPInt { // and leak it. ValLarge = O; } - HoldsLarge = true; } LLVM_ATTRIBUTE_ALWAYS_INLINE explicit DynamicAPInt( const detail::SlowDynamicAPInt &Val) - : ValLarge(Val), HoldsLarge(true) {} - LLVM_ATTRIBUTE_ALWAYS_INLINE bool isSmall() const { return !HoldsLarge; } - LLVM_ATTRIBUTE_ALWAYS_INLINE bool isLarge() const { return HoldsLarge; } + : ValLarge(Val) {} + constexpr bool isSmall() const { return ValLarge.Val.BitWidth == 0; } + constexpr bool isLarge() const { return !isSmall(); } /// Get the stored value. For getSmall/Large, /// the stored value should be small/large. LLVM_ATTRIBUTE_ALWAYS_INLINE int64_t getSmall() const { @@ -105,14 +103,17 @@ class DynamicAPInt { public: LLVM_ATTRIBUTE_ALWAYS_INLINE explicit DynamicAPInt(int64_t Val) - : ValSmall(Val), HoldsLarge(false) {} + : ValSmall(Val) { + ValLarge.Val.BitWidth = 0; + } LLVM_ATTRIBUTE_ALWAYS_INLINE DynamicAPInt() : DynamicAPInt(0) {} LLVM_ATTRIBUTE_ALWAYS_INLINE ~DynamicAPInt() { if (LLVM_UNLIKELY(isLarge())) ValLarge.detail::SlowDynamicAPInt::~SlowDynamicAPInt(); } LLVM_ATTRIBUTE_ALWAYS_INLINE DynamicAPInt(const DynamicAPInt &O) - : ValSmall(O.ValSmall), HoldsLarge(false) { + : ValSmall(O.ValSmall) { + ValLarge.Val.BitWidth = 0; if (LLVM_UNLIKELY(O.isLarge())) initLarge(O.ValLarge); } diff --git a/llvm/include/llvm/ADT/SlowDynamicAPInt.h b/llvm/include/llvm/ADT/SlowDynamicAPInt.h index 1678bda046fec..cda5f39eb98c3 100644 --- a/llvm/include/llvm/ADT/SlowDynamicAPInt.h +++ b/llvm/include/llvm/ADT/SlowDynamicAPInt.h @@ -21,6 +21,10 @@ #include "llvm/ADT/APInt.h" #include "llvm/Support/raw_ostream.h" +namespace llvm { +class DynamicAPInt; +} // namespace llvm + namespace llvm::detail { /// A simple class providing dynamic arbitrary-precision arithmetic. Internally, /// it stores an APInt, whose width is doubled whenever an overflow occurs at a @@ -69,6 +73,9 @@ class SlowDynamicAPInt { /// Overload to compute a hash_code for a SlowDynamicAPInt value. friend hash_code hash_value(const SlowDynamicAPInt &X); // NOLINT + // Make DynamicAPInt a friend so it can access Val directly. + friend DynamicAPInt; + unsigned getBitWidth() const { return Val.getBitWidth(); } void print(raw_ostream &OS) const; From 0ed017abfe1006217039ae9e3ec5be46c8caec80 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Fri, 5 Jul 2024 20:10:15 +0100 Subject: [PATCH 2/4] DynamicAPInt: fix inline issue; address review --- llvm/include/llvm/ADT/DynamicAPInt.h | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/llvm/include/llvm/ADT/DynamicAPInt.h b/llvm/include/llvm/ADT/DynamicAPInt.h index 03325d3313b5a..aaf476ed0f248 100644 --- a/llvm/include/llvm/ADT/DynamicAPInt.h +++ b/llvm/include/llvm/ADT/DynamicAPInt.h @@ -36,8 +36,11 @@ namespace llvm { /// performance slowdown. /// /// When isLarge returns true, a SlowMPInt is held in the union. If isSmall -/// returns true, the int64_t is held. Using std::variant instead would lead to -/// significantly worse performance. +/// returns true, the int64_t is held. We don't have a separate field for +/// indicating this, and instead "steal" memory from ValLarge when it is not in +/// use because we know that the memory layout of APInt is such that BitWidth +/// doesn't overlap with ValSmall (see static_assert_layout). Using std::variant +/// instead would lead to significantly worse performance. class DynamicAPInt { union { int64_t ValSmall; @@ -70,8 +73,12 @@ class DynamicAPInt { LLVM_ATTRIBUTE_ALWAYS_INLINE explicit DynamicAPInt( const detail::SlowDynamicAPInt &Val) : ValLarge(Val) {} - constexpr bool isSmall() const { return ValLarge.Val.BitWidth == 0; } - constexpr bool isLarge() const { return !isSmall(); } + LLVM_ATTRIBUTE_ALWAYS_INLINE constexpr bool isSmall() const { + return ValLarge.Val.BitWidth == 0; + } + LLVM_ATTRIBUTE_ALWAYS_INLINE constexpr bool isLarge() const { + return !isSmall(); + } /// Get the stored value. For getSmall/Large, /// the stored value should be small/large. LLVM_ATTRIBUTE_ALWAYS_INLINE int64_t getSmall() const { @@ -204,6 +211,11 @@ class DynamicAPInt { friend hash_code hash_value(const DynamicAPInt &x); // NOLINT + void static_assert_layout() { // NOLINT + static_assert(offsetof(DynamicAPInt, ValSmall) != + offsetof(DynamicAPInt, ValLarge.Val.BitWidth)); + } + raw_ostream &print(raw_ostream &OS) const; LLVM_DUMP_METHOD void dump() const; }; From 71fcd1e21266c38cfef974142b1f07328626cd43 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Fri, 5 Jul 2024 20:25:33 +0100 Subject: [PATCH 3/4] DynamicAPInt: strengthen static_assert_layout --- llvm/include/llvm/ADT/DynamicAPInt.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/llvm/include/llvm/ADT/DynamicAPInt.h b/llvm/include/llvm/ADT/DynamicAPInt.h index aaf476ed0f248..b2e2d53b8280c 100644 --- a/llvm/include/llvm/ADT/DynamicAPInt.h +++ b/llvm/include/llvm/ADT/DynamicAPInt.h @@ -212,8 +212,11 @@ class DynamicAPInt { friend hash_code hash_value(const DynamicAPInt &x); // NOLINT void static_assert_layout() { // NOLINT - static_assert(offsetof(DynamicAPInt, ValSmall) != - offsetof(DynamicAPInt, ValLarge.Val.BitWidth)); + constexpr size_t ValLargeOff = + offsetof(DynamicAPInt, ValLarge.Val.BitWidth); + constexpr size_t ValSmallOff = offsetof(DynamicAPInt, ValSmall); + constexpr size_t ValSmallSz = sizeof(ValSmall); + static_assert(ValLargeOff >= ValSmallOff + ValSmallSz); } raw_ostream &print(raw_ostream &OS) const; From 0b6272efc55c18a27092390ffb2660fc2316fe01 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Sat, 6 Jul 2024 11:36:00 +0100 Subject: [PATCH 4/4] DynamicAPInt: move static_assert_layout to cpp --- llvm/include/llvm/ADT/DynamicAPInt.h | 8 +------- llvm/lib/Support/DynamicAPInt.cpp | 8 ++++++++ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/llvm/include/llvm/ADT/DynamicAPInt.h b/llvm/include/llvm/ADT/DynamicAPInt.h index b2e2d53b8280c..2f11f91f81e3b 100644 --- a/llvm/include/llvm/ADT/DynamicAPInt.h +++ b/llvm/include/llvm/ADT/DynamicAPInt.h @@ -211,13 +211,7 @@ class DynamicAPInt { friend hash_code hash_value(const DynamicAPInt &x); // NOLINT - void static_assert_layout() { // NOLINT - constexpr size_t ValLargeOff = - offsetof(DynamicAPInt, ValLarge.Val.BitWidth); - constexpr size_t ValSmallOff = offsetof(DynamicAPInt, ValSmall); - constexpr size_t ValSmallSz = sizeof(ValSmall); - static_assert(ValLargeOff >= ValSmallOff + ValSmallSz); - } + void static_assert_layout(); // NOLINT raw_ostream &print(raw_ostream &OS) const; LLVM_DUMP_METHOD void dump() const; diff --git a/llvm/lib/Support/DynamicAPInt.cpp b/llvm/lib/Support/DynamicAPInt.cpp index cae034cf6da6f..bfcb97e0cc96a 100644 --- a/llvm/lib/Support/DynamicAPInt.cpp +++ b/llvm/lib/Support/DynamicAPInt.cpp @@ -18,6 +18,14 @@ hash_code llvm::hash_value(const DynamicAPInt &X) { return detail::hash_value(X.getLarge()); } +void DynamicAPInt::static_assert_layout() { + constexpr size_t ValLargeOffset = + offsetof(DynamicAPInt, ValLarge.Val.BitWidth); + constexpr size_t ValSmallOffset = offsetof(DynamicAPInt, ValSmall); + constexpr size_t ValSmallSize = sizeof(ValSmall); + static_assert(ValLargeOffset >= ValSmallOffset + ValSmallSize); +} + raw_ostream &DynamicAPInt::print(raw_ostream &OS) const { if (isSmall()) return OS << ValSmall;