[PATCH] D144686: [CodeGen] Remove the template specialization of `Address` that stores alignment information into pointers

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: aeubanks, efriedma, rjmccall.
ahatanak added a project: clang.
Herald added a project: All.
ahatanak requested review of this revision.

This fixes a bug introduced in https://reviews.llvm.org/D142584. The patch reduced the number of bits used for alignment from 6 bits to 5 bits, which made it impossible to encode the maximum allowed alignment in llvm (1 << 32).

This reverts b1613f05ae0ce4efc6b6475ea4459957ebcb0150 <https://reviews.llvm.org/rGb1613f05ae0ce4efc6b6475ea4459957ebcb0150>. The change was made in an attempt to reduce memory consumption by storing the alignment information into pointers, but it turns out it doesn't make much difference.

https://llvm-compile-time-tracker.com/compare.php?from=998ad085e865f2e5acc589d6bee0e3379042da2e&to=5de4a1989c474f37ac03f20ccb0aef50f6e3b854&stat=max-rss


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144686

Files:
  clang/lib/CodeGen/Address.h


Index: clang/lib/CodeGen/Address.h
===================================================================
--- clang/lib/CodeGen/Address.h
+++ clang/lib/CodeGen/Address.h
@@ -25,12 +25,7 @@
 // Indicates whether a pointer is known not to be null.
 enum KnownNonNull_t { NotKnownNonNull, KnownNonNull };
 
-// We try to save some space by using 6 bits over two PointerIntPairs to store
-// the alignment. However, some arches don't support 3 bits in a PointerIntPair
-// so we fallback to storing the alignment separately.
-template <typename T, bool = alignof(llvm::Value *) >= 8> class AddressImpl {};
-
-template <typename T> class AddressImpl<T, false> {
+class AddressImpl {
   llvm::PointerIntPair<llvm::Value *, 1, bool> PointerAndKnownNonNull;
   llvm::Type *ElementType;
   CharUnits Alignment;
@@ -51,45 +46,9 @@
   void setKnownNonNull() { PointerAndKnownNonNull.setInt(true); }
 };
 
-template <typename T> class AddressImpl<T, true> {
-  // Int portion stores the non-null bit and the upper 2 bits of the log of the
-  // alignment.
-  llvm::PointerIntPair<llvm::Value *, 3, unsigned> Pointer;
-  // Int portion stores lower 3 bits of the log of the alignment.
-  llvm::PointerIntPair<llvm::Type *, 3, unsigned> ElementType;
-
-public:
-  AddressImpl(llvm::Value *Pointer, llvm::Type *ElementType,
-              CharUnits Alignment, KnownNonNull_t IsKnownNonNull)
-      : Pointer(Pointer), ElementType(ElementType) {
-    if (Alignment.isZero()) {
-      this->Pointer.setInt(IsKnownNonNull << 2);
-      return;
-    }
-    // Currently the max supported alignment is exactly 1 << 32 and is
-    // guaranteed to be a power of 2, so we can store the log of the alignment
-    // into 5 bits.
-    assert(Alignment.isPowerOfTwo() && "Alignment cannot be zero");
-    auto AlignLog = llvm::Log2_64(Alignment.getQuantity());
-    assert(AlignLog < (1 << 5) && "cannot fit alignment into 5 bits");
-    this->Pointer.setInt(IsKnownNonNull << 2 | AlignLog >> 3);
-    this->ElementType.setInt(AlignLog & 7);
-  }
-  llvm::Value *getPointer() const { return Pointer.getPointer(); }
-  llvm::Type *getElementType() const { return ElementType.getPointer(); }
-  CharUnits getAlignment() const {
-    unsigned AlignLog = ((Pointer.getInt() & 0x3) << 3) | ElementType.getInt();
-    return CharUnits::fromQuantity(CharUnits::QuantityType(1) << AlignLog);
-  }
-  KnownNonNull_t isKnownNonNull() const {
-    return (KnownNonNull_t)(!!(Pointer.getInt() & 0x4));
-  }
-  void setKnownNonNull() { Pointer.setInt(Pointer.getInt() | 0x4); }
-};
-
 /// An aligned address.
 class Address {
-  AddressImpl<void> A;
+  AddressImpl A;
 
 protected:
   Address(std::nullptr_t)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144686.500010.patch
Type: text/x-patch
Size: 2668 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230224/e33131f8/attachment.bin>