Merge pull request #72485 from eeckstein/node-bits

Some improvements for inline bitfields in SILNode, SILBasicBlock and Operand

Author: eeckstein

SwiftCompilerSources/Sources/Optimizer/DataStructures/Set.swift

@@ -176,3 +176,55 @@ struct InstructionSet : IntrusiveSet {
     context.freeNodeSet(bridged)
   }
 }
+
+/// A set of operands.
+///
+/// This is an extremely efficient implementation which does not need memory
+/// allocations or hash lookups.
+///
+/// This type should be a move-only type, but unfortunately we don't have move-only
+/// types yet. Therefore it's needed to call `deinitialize()` explicitly to
+/// destruct this data structure, e.g. in a `defer {}` block.
+struct OperandSet : IntrusiveSet {
+
+  private let context: BridgedPassContext
+  private let bridged: BridgedOperandSet
+
+  init(_ context: some Context) {
+    self.context = context._bridged
+    self.bridged = self.context.allocOperandSet()
+  }
+
+  func contains(_ operand: Operand) -> Bool {
+    bridged.contains(operand.bridged)
+  }
+
+  /// Returns true if `inst` was not contained in the set before inserting.
+  @discardableResult
+  mutating func insert(_ operand: Operand) -> Bool {
+    bridged.insert(operand.bridged)
+  }
+
+  mutating func erase(_ operand: Operand) {
+    bridged.erase(operand.bridged)
+  }
+
+  var description: String {
+    let function = bridged.getFunction().function
+    var d = "{\n"
+    for inst in function.instructions {
+      for op in inst.operands {
+        if contains(op) {
+          d += op.description
+        }
+      }
+    }
+    d += "}\n"
+    return d
+  }
+
+  /// TODO: once we have move-only types, make this a real deinit.
+  mutating func deinitialize() {
+    context.freeOperandSet(bridged)
+  }
+}

SwiftCompilerSources/Sources/Optimizer/DataStructures/Worklist.swift

@@ -75,3 +75,4 @@ struct Worklist<Set: IntrusiveSet> : CustomStringConvertible, NoReflectionChildr
 typealias BasicBlockWorklist = Worklist<BasicBlockSet>
 typealias InstructionWorklist = Worklist<InstructionSet>
 typealias ValueWorklist = Worklist<ValueSet>
+typealias OperandWorklist = Worklist<OperandSet>

SwiftCompilerSources/Sources/SIL/Operand.swift

@@ -14,7 +14,7 @@ import SILBridging
 
 /// An operand of an instruction.
 public struct Operand : CustomStringConvertible, NoReflectionChildren {
-  fileprivate let bridged: BridgedOperand
+  public let bridged: BridgedOperand
 
   public init(bridged: BridgedOperand) {
     self.bridged = bridged

include/swift/Basic/Require.h

@@ -0,0 +1,34 @@
+//===--- Require.h ----------------------------------------------*- C++ -*-===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines swift_unreachable, which provides the
+//  functionality of llvm_unreachable without necessarily depending on
+//  the LLVM support libraries.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SWIFT_BASIC_REQUIRE_H
+#define SWIFT_BASIC_REQUIRE_H
+
+#include "llvm/Support/raw_ostream.h"
+
+/// Checks the `condition` and if it's false abort with `message`.
+/// In contrast to `assert` and similar functions, `require` works in
+/// no-assert builds the same way as in debug builds.
+inline void require(bool condition, const char *message) {
+  if (!condition) {
+    llvm::errs() << message << '\n';
+    abort();
+  }
+}
+
+#endif

include/swift/SIL/SILBasicBlock.h

@@ -130,16 +130,13 @@ public SwiftObjectHeader {
   ///    DD and EEE are uninitialized
   ///
   /// See also: SILBitfield::bitfieldID, SILFunction::currentBitfieldID.
-  int64_t lastInitializedBitfieldID = 0;
+  uint64_t lastInitializedBitfieldID = 0;
 
   // Used by `BasicBlockBitfield`.
   unsigned getCustomBits() const { return customBits; }
   // Used by `BasicBlockBitfield`.
   void setCustomBits(unsigned value) { customBits = value; }
 
-  // Used by `BasicBlockBitfield`.
-  enum { numCustomBits = std::numeric_limits<CustomBitsType>::digits };
-
   friend struct llvm::ilist_traits<SILBasicBlock>;
 
   SILBasicBlock();
@@ -155,7 +152,8 @@ public SwiftObjectHeader {
 
   ~SILBasicBlock();
 
-  bool isMarkedAsDeleted() const { return lastInitializedBitfieldID < 0; }
+  enum { numCustomBits = std::numeric_limits<CustomBitsType>::digits };
+  enum { maxBitfieldID = std::numeric_limits<uint64_t>::max() };
 
   /// Gets the ID (= index in the function's block list) of the block.
   ///

include/swift/SIL/SILBitfield.h

@@ -17,6 +17,7 @@
 #ifndef SWIFT_SIL_SILBITFIELD_H
 #define SWIFT_SIL_SILBITFIELD_H
 
+#include "swift/Basic/Require.h"
 #include "swift/SIL/SILFunction.h"
 
 namespace swift {
@@ -30,7 +31,7 @@ template <class Impl, class T> class SILBitfield {
   /// that the bits of that block are not initialized yet.
   /// See also: SILBasicBlock::lastInitializedBitfieldID,
   ///           SILFunction::currentBitfieldID
-  int64_t bitfieldID;
+  uint64_t bitfieldID;
 
   short startBit;
   short endBit;
@@ -55,11 +56,13 @@ template <class Impl, class T> class SILBitfield {
       parent(parent),
       function(function) {
     assert(size > 0 && "bit field size must be > 0");
-    assert(endBit <= T::numCustomBits && "too many/large bit fields allocated in function");
+    require(endBit <= T::numCustomBits,
+            "too many/large bit fields allocated in function");
     assert((!parent || bitfieldID > parent->bitfieldID) &&
            "BasicBlockBitfield indices are not in order");
+    require(function->currentBitfieldID < T::maxBitfieldID,
+            "currentBitfieldID overflow");
     ++function->currentBitfieldID;
-    assert(function->currentBitfieldID != 0 && "currentBitfieldID overflow");
   }
 
   SILBitfield(const SILBitfield &) = delete;
@@ -85,9 +88,6 @@ template <class Impl, class T> class SILBitfield {
     unsigned clearMask = mask;
     if (bitfieldID > entity->lastInitializedBitfieldID) {
 
-      if (entity->isMarkedAsDeleted())
-        return;
-
       // The bitfield is not initialized yet in this block.
       // Initialize the bitfield, and also initialize all parent bitfields,
       // which are not initialized, yet. Example:

include/swift/SIL/SILFunction.h

@@ -294,7 +294,7 @@ class SILFunction
   /// A monotonically increasing ID which is incremented whenever a
   /// BasicBlockBitfield, NodeBitfield, or OperandBitfield is constructed.  For
   /// details see SILBitfield::bitfieldID;
-  int64_t currentBitfieldID = 1;
+  uint64_t currentBitfieldID = 1;
 
   /// Unique identifier for vector indexing and deterministic sorting.
   /// May be reused when zombie functions are recovered.

include/swift/SIL/SILNode.h

@@ -126,6 +126,9 @@ class alignas(8) SILNode :
   enum { NumAllocRefTailTypesBits = 4 };
   enum { NumMarkDependenceKindBits = 2 };
 
+  enum { numCustomBits = 20 };
+  enum { maxBitfieldID = std::numeric_limits<uint64_t>::max() >> numCustomBits };
+
 protected:
   friend class SILInstruction;
   template <class, class> friend class SILBitfield;
@@ -135,11 +138,7 @@ class alignas(8) SILNode :
 
   uint8_t kind;
 
-  // Used by `NodeBitfield`.
-  enum { numCustomBits = 8 };
-
-  // Used by `NodeBitfield`.
-  uint8_t customBits;
+  bool deleted = false;
 
   // Part of SILInstruction's debug location. Together with
   // `SILInstruction::locationStorage` this forms the SILLocation.
@@ -364,6 +363,9 @@ class alignas(8) SILNode :
 
   //===---------------------- end of shared fields ------------------------===//
 
+  // Used by `NodeBitfield`.
+  uint64_t customBits : numCustomBits;
+
   /// The NodeBitfield ID of the last initialized bitfield in `customBits`.
   /// Example:
   ///
@@ -377,20 +379,19 @@ class alignas(8) SILNode :
   /// -> AAA, BB and C are initialized,
   ///    DD and EEE are uninitialized
   ///
-  /// If the ID is negative, it means that the node (in case it's an instruction)
-  /// is deleted, i.e. it does not belong to the function anymore. Conceptually
-  /// this results in setting all bitfields to zero, which e.g. "removes" the
-  /// node from all NodeSets.
+  /// The size of lastInitializedBitfieldID should be more than 32 bits to
+  /// absolutely avoid an overflow.
   ///
   /// See also: SILBitfield::bitfieldID, SILFunction::currentBitfieldID.
-  int64_t lastInitializedBitfieldID = 0;
+  uint64_t lastInitializedBitfieldID : (64 - numCustomBits);
 
 private:
   SwiftMetatype getSILNodeMetatype(SILNodeKind kind);
 
 protected:
   SILNode(SILNodeKind kind) : SwiftObjectHeader(getSILNodeMetatype(kind)),
-                              kind((uint8_t)kind) {
+                              kind((uint8_t)kind),
+                              customBits(0), lastInitializedBitfieldID(0) {
     _sharedUInt8_private.opaque = 0;
     _sharedUInt32_private.opaque = 0;
   }
@@ -442,11 +443,8 @@ class alignas(8) SILNode :
     lastInitializedBitfieldID = 0;
   }
 
-  void markAsDeleted() {
-    lastInitializedBitfieldID = -1;
-  }
-
-  bool isMarkedAsDeleted() const { return lastInitializedBitfieldID < 0; }
+  void markAsDeleted() { deleted = true; }
+  bool isMarkedAsDeleted() const { return deleted; }
 
   static SILNode *instAsNode(SILInstruction *inst);
   static const SILNode *instAsNode(const SILInstruction *inst);