[NFC] Store Address's alignment into PointerIntPairs

[aeubanks]

Summary

This mitigates the extra memory caused by D115725.

[aeubanks]

update

[rnk]

Thanks! Does this have any impact on binary size, as a proxy for more thorough performance testing?

[aeubanks]

# no asserts, no debuginfo, -O3
$ du -b clang-stable clang-patched
83539528        clang-stable
83528456        clang-patched

clang's code size actually goes down with this patch

llvm-compile-time-tracker:
https://llvm-compile-time-tracker.com/compare.php?from=a9bfb4c4f48d9434918374566d5780c97a805bcc&to=e7e52fd42632a5a8d57520ce5ca2596f7c3be2a1&stat=instructions

there is a little bit of red for compile times, and a little bit of green for max-rss, I think this tradeoff is fine

[nikic]

clang/lib/CodeGen/Address.h

30

Are we guaranteed 3 bits even on 32-bit architectures?

44

Why can we assume this?

[nikic]

clang/lib/CodeGen/Address.h

44

Ah, the alignment is 64-bits, so the log2 is 0-63, which fits exactly into 6 bits. That's handy :)

[aeubanks]

clang/lib/CodeGen/Address.h

30

Apparently not, a static assert fires for a 32-bit build of clang. I was assuming 3 was fine based on the PointerIntPair comments.

/// PointerIntPair - This class implements a pair of a pointer and small
/// integer.  It is designed to represent this in the space required by one
/// pointer by bitmangling the integer into the low part of the pointer.  This
/// can only be done for small integers: typically up to 3 bits, but it depends
/// on the number of bits available according to PointerLikeTypeTraits for the
/// type.

I suppose we could have a separate 32-bit vs 64-bit implementation of Address, although that's not very nice.

[dblaikie]

clang/lib/CodeGen/Address.h

30

You can get more spare bits by overaligning the type that you're pointing to, if that's suitable/workable. We do that in a few places in LLVM for this sort of reason.

[nikic]

clang/lib/CodeGen/Address.h

30

As we're storing pointers to core LLVM types here (Value and Type), I don't think it's possible to over-align them.

[dblaikie]

clang/lib/CodeGen/Address.h

30

Ah, right :/

[rnk]

clang/lib/CodeGen/Address.h

30

Oh well, this probably won't work without some truly squirelly hackery. We store four bits of alignment locally, and make all-zeros a sentinel value to store large alignments out of line. It's not worth it.

[aeubanks]

only use PointerIntPair when alignof(Value*) >= 8

[nikic]

Approach looks reasonable to me.

clang/lib/CodeGen/Address.h

30

Why do we need the extra T parameter?

65

This should probably be 1UL to avoid UB for large alignments.

[aeubanks]

address comments

clang/lib/CodeGen/Address.h

30

without it we end up instantiating AddressImpl<true> unconditionally (because all template parameters are known?) which static_asserts in 32 bit builds

[nikic]

LG from my side

[rnk]

lgtm

BTW, as a follow-on cleanup, which I may attempt, I think we could convert this alignment from CharUnits to llvm::Align. The vast majority of the getAlignment callers feed it to CGBuilder::CreateAlignedLoad/Store:
http://llvm-cs.pcc.me.uk/tools/clang/lib/CodeGen/Address.h/rgetAlignment
Those call IRBuilder::CreateAlignedLoad/Store, which accept MaybeAlign.

[LegalizeAdulthood]

clang/lib/CodeGen/Address.h

65

This is causing warnings to be emitted:

clang\lib\CodeGen\Address.h(69,1): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) [D:\legalize\llvm\llvm-project\build\tools\clang\lib\CodeGen\obj.clangCodeGen.vcxproj]

[aeubanks]

clang/lib/CodeGen/Address.h

65

Should be fixed by eee97f1617c94b8

clang/lib/CodeGen/Address.h

 //===-- Address.h - An aligned address -------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This class provides a simple wrapper for a pair of a pointer and an
 // alignment.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 #define LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 
-#include "llvm/IR/Constants.h"
 #include "clang/AST/CharUnits.h"
+#include "llvm/ADT/PointerIntPair.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/Support/MathExtras.h"
 
 namespace clang {
 namespace CodeGen {
 
 /// An aligned address.
 class Address {
-  llvm::Value *Pointer;
-  llvm::Type *ElementType;
-  CharUnits Alignment;
+  llvm::PointerIntPair<llvm::Value *, 3, unsigned> Pointer;
+  llvm::PointerIntPair<llvm::Type *, 3, unsigned> ElementType;
+  // CharUnits Alignment;
 
 protected:
   Address(std::nullptr_t) : Pointer(nullptr), ElementType(nullptr) {}
 
 public:
   Address(llvm::Value *pointer, llvm::Type *elementType, CharUnits alignment)
-      : Pointer(pointer), ElementType(elementType), Alignment(alignment) {
+      : Pointer(pointer), ElementType(elementType) {
     assert(pointer != nullptr && "Pointer cannot be null");
     assert(elementType != nullptr && "Element type cannot be null");
     assert(llvm::cast<llvm::PointerType>(pointer->getType())
                ->isOpaqueOrPointeeTypeMatches(elementType) &&
            "Incorrect pointer element type");
-    assert(!alignment.isZero() && "Alignment cannot be zero");
+    assert(alignment.isPowerOfTwo() && "Alignment cannot be zero");
+    auto AlignLog = llvm::Log2_64(alignment.getQuantity());
+    assert(AlignLog < (1 << 6) && "cannot fit alignment into 6 bits");
+    Pointer.setInt(AlignLog >> 3);
+    ElementType.setInt(AlignLog & 7);
   }
 
   // Deprecated: Use constructor with explicit element type instead.
   Address(llvm::Value *Pointer, CharUnits Alignment)
       : Address(Pointer, Pointer->getType()->getPointerElementType(),
                 Alignment) {}
 
   static Address invalid() { return Address(nullptr); }
-  bool isValid() const { return Pointer != nullptr; }
+  bool isValid() const { return Pointer.getPointer() != nullptr; }
 
   llvm::Value *getPointer() const {
     assert(isValid());
-    return Pointer;
+    return Pointer.getPointer();
   }
 
   /// Return the type of the pointer value.
   llvm::PointerType *getType() const {
     return llvm::cast<llvm::PointerType>(getPointer()->getType());
   }
 
   /// Return the type of the values stored in this address.
   llvm::Type *getElementType() const {
     assert(isValid());
-    return ElementType;
+    return ElementType.getPointer();
   }
 
   /// Return the address space that this address resides in.
   unsigned getAddressSpace() const {
     return getType()->getAddressSpace();
   }
 
   /// Return the IR name of the pointer value.
   llvm::StringRef getName() const {
     return getPointer()->getName();
   }
 
   /// Return the alignment of this pointer.
   CharUnits getAlignment() const {
     assert(isValid());
-    return Alignment;
+    unsigned AlignLog = (Pointer.getInt() << 3) | ElementType.getInt();
+    return CharUnits::fromQuantity(1 << AlignLog);
   }
 
   /// Return address with different pointer, but same element type and
   /// alignment.
   Address withPointer(llvm::Value *NewPointer) const {
-    return Address(NewPointer, ElementType, Alignment);
+    return Address(NewPointer, ElementType.getPointer(), getAlignment());
   }
 
   /// Return address with different alignment, but same pointer and element
   /// type.
   Address withAlignment(CharUnits NewAlignment) const {
-    return Address(Pointer, ElementType, NewAlignment);
+    return Address(Pointer.getPointer(), ElementType.getPointer(),
+                   NewAlignment);
   }
 };
 
 /// A specialization of Address that requires the address to be an
 /// LLVM Constant.
 class ConstantAddress : public Address {
   ConstantAddress(std::nullptr_t) : Address(nullptr) {}