[Mlir-commits] [mlir] 6304c08 - [mlir] Store the flag for dynamic operand storage in the low bits

[River Riddle]

Author: River Riddle
Date: 2021-05-06T12:45:35-07:00
New Revision: 6304c0836a4dd2d77373d45716092b2a088fa948

URL: https://github.com/llvm/llvm-project/commit/6304c0836a4dd2d77373d45716092b2a088fa948
DIFF: https://github.com/llvm/llvm-project/commit/6304c0836a4dd2d77373d45716092b2a088fa948.diff

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

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.

Differential Revision: https://reviews.llvm.org/D101969

Added: 
    

Modified: 
    mlir/include/mlir/IR/OperationSupport.h
    mlir/lib/IR/OperationSupport.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 7cfb979ea2ea..e7bcacf79a0c 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -480,6 +480,7 @@ 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();
@@ -490,12 +491,12 @@ struct TrailingOperandStorage final
     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
@@ -537,9 +538,11 @@ class OperandStorage final
   }
 
 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
@@ -555,27 +558,25 @@ class OperandStorage final
   /// 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.

diff  --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index fc7cf9310a83..60910f7f35de 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -224,7 +224,7 @@ void OperationState::addRegions(
 //===----------------------------------------------------------------------===//
 
 detail::OperandStorage::OperandStorage(Operation *owner, ValueRange values)
-    : representation(0) {
+    : inlineStorage() {
   auto &inlineStorage = getInlineStorage();
   inlineStorage.numOperands = inlineStorage.capacity = values.size();
   auto *operandPtrBegin = getTrailingObjects<OpOperand>();
@@ -380,8 +380,7 @@ MutableArrayRef<OpOperand> detail::OperandStorage::resize(Operation *owner,
   }
 
   // Update the storage representation to use the new dynamic storage.
-  representation = reinterpret_cast<intptr_t>(newStorage);
-  representation |= DynamicStorageBit;
+  dynamicStorage.setPointerAndInt(newStorage, true);
   return newOperands;
 }