[clang] [clang] LazyOffsetPtr: Use native pointer width (PR #111995)

A. Wilcox via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 16 17:48:13 PDT 2024


================
@@ -326,25 +326,25 @@ struct LazyOffsetPtr {
   ///
   /// If the low bit is clear, a pointer to the AST node. If the low
   /// bit is set, the upper 63 bits are the offset.
-  mutable uint64_t Ptr = 0;
+  mutable uintptr_t Ptr = 0;
 
 public:
   LazyOffsetPtr() = default;
-  explicit LazyOffsetPtr(T *Ptr) : Ptr(reinterpret_cast<uint64_t>(Ptr)) {}
+  explicit LazyOffsetPtr(T *Ptr) : Ptr(reinterpret_cast<uintptr_t>(Ptr)) {}
 
-  explicit LazyOffsetPtr(uint64_t Offset) : Ptr((Offset << 1) | 0x01) {
-    assert((Offset << 1 >> 1) == Offset && "Offsets must require < 63 bits");
+  explicit LazyOffsetPtr(uintptr_t Offset) : Ptr((Offset << 1) | 0x01) {
+    assert((Offset << 1 >> 1) == Offset && "Offsets must fit in addressable bits");
     if (Offset == 0)
       Ptr = 0;
   }
 
   LazyOffsetPtr &operator=(T *Ptr) {
-    this->Ptr = reinterpret_cast<uint64_t>(Ptr);
+    this->Ptr = reinterpret_cast<uintptr_t>(Ptr);
     return *this;
   }
 
-  LazyOffsetPtr &operator=(uint64_t Offset) {
-    assert((Offset << 1 >> 1) == Offset && "Offsets must require < 63 bits");
+  LazyOffsetPtr &operator=(uintptr_t Offset) {
----------------
awilfox wrote:

Okay.  But it is still invalid to `reinterpret_cast` a `uint64_t` to a pointer on 32-bit BE systems, and that would still leave that as a segfault.

[C++23 draft N4950](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/n4950.pdf), ยง7.6.1.10 number 5:
> A value of integral type or enumeration type can be explicitly converted to a pointer. A pointer converted to an integer of sufficient size (if any such exists on the implementation) and back to the same pointer type will have its original value; mappings between pointers and integers are otherwise implementation-defined.

uint64_t is not the sufficient size for a 32-bit BE system.  How should we proceed?

* I suppose a union could work, though it would probably be ugly.
* Alternatively, there could be two fields (a `uint64_t Offset` and a `uintptr_t Ptr`), but that would unconditionally add 8 bytes per `LazyOffsetPtr`.  I'm not completely familiar with how many live `LazyOffsetPtr` objects might be present in a single invocation of Clang and how that would affect memory usage.
* If there is a different way, I'd be happy to implement that as well.

https://github.com/llvm/llvm-project/pull/111995


More information about the cfe-commits mailing list