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

[Jessica Clarke via cfe-commits]

================
@@ -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) {
----------------
jrtc27 wrote:

> > That doesn't work for getAddressOfPointer
> 
> Maybe we can just define away the need for `getAddressOfPointer`.

If you can do that then the current code works just fine :)



> > so we need to preserve at least 64 bits in this structure.
> 
> Hmm. We're only really preserving 63 bits here, so that might be a problem.
> 
> > That doesn't work for getAddressOfPointer
> 
> Maybe we can just define away the need for `getAddressOfPointer`.
> 
> > (and doesn't work for CHERI either, but there are many instances of that idiom in Clang to fix...).
> 
> Is the CHERI problem just the assumption that `uint64_t` is at least as wide as `uintptr_t`, or is there a broader issue with round-tripping through an integer type at all? At any rate, if we need this type to be implemented substantially differently on CHERI, that's fine, but I'd like to not regress performance on other platforms because of it.

The problem is that uint64_t cannot hold a pointer on CHERI, but uintptr_t can round-trip just fine. The approach I've used downstream in the past when prototyping self-hosted CHERI LLVM was to use uintptr_t when its range is at least uint64_t's and to fall back on uint64_t otherwise (effectively using uint64_t for ILP32 and uintptr_t otherwise). Though if and when I ever get back to cleaning all that up and upstreaming it I'd like to abstract it into a reusable helper rather than replicating the slightly messy std::conditional_t pattern.

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