[mlir] Store the flag for dynamic operand storage in the low bits

[rriddle]

Summary

It is currently stored in the high bits, which is disallowed on certain
platforms (e.g. android). This revision switches the representation to use
the low bits instead, fixing crashes/breakages on those platforms.

[mehdi_amini]

mlir/include/mlir/IR/OperationSupport.h

483

Can you just use initializer on the members or it isn't possible with bitfields?

[rriddle]

mlir/include/mlir/IR/OperationSupport.h

483

Yeah, it isn't possible for bitfields until C++20.

[imaihal]

Hi, @rriddle , I got many testing errors after this patch on Systems Z (Big-endian machine)(Triple = "s390x-ibm-linux", CPU = "z14").
It seems I can avoid the errors by changing the bit order (TrailingOperandStorage() : numOperands(0), capacity(0), reserved(0) {}) , but I'm not sure why.
Do you have any suggestion?

mlir/include/mlir/IR/OperationSupport.h

 //===----------------------------------------------------------------------===//
 // OperandStorage
 //===----------------------------------------------------------------------===//
 
 namespace detail {
 /// This class contains the information for a trailing operand storage.
 struct TrailingOperandStorage final
     : public llvm::TrailingObjects<TrailingOperandStorage, OpOperand> {
+  TrailingOperandStorage() : reserved(0), capacity(0), numOperands(0) {}
   ~TrailingOperandStorage() {
     for (auto &operand : getOperands())
       operand.~OpOperand();
   }
 
   /// Return the operands held by this storage.
   MutableArrayRef<OpOperand> getOperands() {
     return {getTrailingObjects<OpOperand>(), numOperands};
   }
 
-  /// The number of operands within the storage.
-  unsigned numOperands;
-  /// The total capacity number of operands that the storage can hold.
-  unsigned capacity : 31;
   /// We reserve a range of bits for use by the operand storage.
   unsigned reserved : 1;
+  /// The total capacity number of operands that the storage can hold.
+  unsigned capacity : 31;
+  /// The number of operands within the storage.
+  unsigned numOperands;
 };
 
 /// This class handles the management of operation operands. Operands are
 /// stored either in a trailing array, or a dynamically resizable vector.
 class OperandStorage final
     : private llvm::TrailingObjects<OperandStorage, OpOperand> {
 public:
   OperandStorage(Operation *owner, ValueRange values);
public:
   unsigned size() { return getStorage().numOperands; }
 
   /// Returns the additional size necessary for allocating this object.
   static size_t additionalAllocSize(unsigned numOperands) {
     return additionalSizeToAlloc<OpOperand>(numOperands);
   }
 
 private:
-  enum : uint64_t {
-    /// The bit used to mark the storage as dynamic.
-    DynamicStorageBit = 1ull << 63ull
+  /// Pointer type traits for the storage pointer that ensures that we use the
+  /// lowest bit for the storage pointer.
+  struct StoragePointerLikeTypeTraits
+      : llvm::PointerLikeTypeTraits<TrailingOperandStorage *> {
+    static constexpr int NumLowBitsAvailable = 1;
   };
 
   /// Resize the storage to the given size. Returns the array containing the new
   /// operands.
   MutableArrayRef<OpOperand> resize(Operation *owner, unsigned newSize);
 
   /// Returns the current internal storage instance.
   TrailingOperandStorage &getStorage() {
     return LLVM_UNLIKELY(isDynamicStorage()) ? getDynamicStorage()
                                              : getInlineStorage();
   }
 
   /// Returns the storage container if the storage is inline.
   TrailingOperandStorage &getInlineStorage() {
     assert(!isDynamicStorage() && "expected storage to be inline");
-    static_assert(sizeof(TrailingOperandStorage) == sizeof(uint64_t),
-                  "inline storage representation must match the opaque "
-                  "representation");
     return inlineStorage;
   }
 
   /// Returns the storage container if this storage is dynamic.
   TrailingOperandStorage &getDynamicStorage() {
     assert(isDynamicStorage() && "expected dynamic storage");
-    uint64_t maskedRepresentation = representation & ~DynamicStorageBit;
-    return *reinterpret_cast<TrailingOperandStorage *>(maskedRepresentation);
+    return *dynamicStorage.getPointer();
   }
 
   /// Returns true if the storage is currently dynamic.
-  bool isDynamicStorage() const { return representation & DynamicStorageBit; }
+  bool isDynamicStorage() const { return dynamicStorage.getInt(); }
 
   /// The current representation of the storage. This is either a
   /// InlineOperandStorage, or a pointer to a InlineOperandStorage.
   union {
     TrailingOperandStorage inlineStorage;
-    uint64_t representation;
+    llvm::PointerIntPair<TrailingOperandStorage *, 1, bool,
+                         StoragePointerLikeTypeTraits>
+        dynamicStorage;
   };
 
   /// This stuff is used by the TrailingObjects template.
   friend llvm::TrailingObjects<OperandStorage, OpOperand>;
 };
 } // end namespace detail
 
 //===----------------------------------------------------------------------===//

mlir/lib/IR/OperationSupport.cpp

for (std::unique_ptr<Region> &region : regions)
     addRegion(std::move(region));
 }
 
 //===----------------------------------------------------------------------===//
 // OperandStorage
 //===----------------------------------------------------------------------===//
 
 detail::OperandStorage::OperandStorage(Operation *owner, ValueRange values)
-    : representation(0) {
+    : inlineStorage() {
   auto &inlineStorage = getInlineStorage();
   inlineStorage.numOperands = inlineStorage.capacity = values.size();
   auto *operandPtrBegin = getTrailingObjects<OpOperand>();
   for (unsigned i = 0, e = inlineStorage.numOperands; i < e; ++i)
     new (&operandPtrBegin[i]) OpOperand(owner, values[i]);
 }
 
 detail::OperandStorage::~OperandStorage() {
MutableArrayRef<OpOperand> detail::OperandStorage::resize(Operation *owner,
   // If the current storage is also dynamic, free it.
   if (isDynamicStorage()) {
     // Workaround false positive in -Wfree-nonheap-object
     auto *mem = &storage;
     free(mem);
   }
 
   // Update the storage representation to use the new dynamic storage.
-  representation = reinterpret_cast<intptr_t>(newStorage);
-  representation |= DynamicStorageBit;
+  dynamicStorage.setPointerAndInt(newStorage, true);
   return newOperands;
 }
 
 //===----------------------------------------------------------------------===//
 // Operation Value-Iterators
 //===----------------------------------------------------------------------===//
 
 //===----------------------------------------------------------------------===//