[DNM] compute the field index with a request instead of caching it in the instruction

include/swift/AST/Decl.h

@@ -4788,8 +4788,12 @@ enum class PropertyWrapperSynthesizedPropertyKind {
 /// VarDecl - 'var' and 'let' declarations.
 class VarDecl : public AbstractStorageDecl {
   friend class NamingPatternRequest;
+  friend class GetFieldIndexRequest;
+  friend class ComputeFieldIndicesRequest;
   NamedPattern *NamingPattern = nullptr;
 
+  int fieldIndex = -1;
+
 public:
   enum class Introducer : uint8_t {
     Let = 0,
@@ -4821,6 +4825,8 @@ class VarDecl : public AbstractStorageDecl {
     return hasName() ? getBaseIdentifier().str() : "_";
   }
 
+  int getFieldIndex() const;
+
   /// Get the type of the variable within its context. If the context is generic,
   /// this will use archetypes.
   Type getType() const;

include/swift/AST/SILOptimizerRequests.h

@@ -87,6 +87,20 @@ class LoweredSILRequest
                                       ASTLoweringDescriptor desc) const;
 };
 
+class ComputeFieldIndicesRequest :
+    public SimpleRequest<ComputeFieldIndicesRequest,
+                         std::tuple<>(NominalTypeDecl *),
+                         RequestFlags::Uncached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  // Evaluation.
+  std::tuple<> evaluate(Evaluator &evaluator, NominalTypeDecl *decl) const;
+};
+
 /// Report that a request of the given kind is being evaluated, so it
 /// can be recorded by the stats reporter.
 template <typename Request>

include/swift/AST/SILOptimizerTypeIDZone.def

@@ -20,3 +20,6 @@ SWIFT_REQUEST(SILOptimizer, ExecuteSILPipelineRequest,
 SWIFT_REQUEST(SILOptimizer, LoweredSILRequest,
               std::unique_ptr<SILModule>(ASTLoweringDescriptor),
               Uncached, NoLocationInfo)
+SWIFT_REQUEST(SILOptimizer, ComputeFieldIndicesRequest,
+              void(NominalTypeDecl *),
+              Cached, NoLocationInfo)

include/swift/SIL/SILInstruction.h

@@ -6265,89 +6265,29 @@ unsigned getFieldIndex(NominalTypeDecl *decl, VarDecl *property);
 /// Precondition: \p decl must be a non-resilient struct or class.
 VarDecl *getIndexedField(NominalTypeDecl *decl, unsigned index);
 
-/// A common base for instructions that require a cached field index.
-///
-/// "Field" is a term used here to refer to the ordered, accessible stored
-/// properties of a class or struct.
-///
-/// The field's ordinal value is the basis of efficiently comparing and sorting
-/// access paths in SIL. For example, whenever a Projection object is created,
-/// it stores the field index. Finding the field index initially requires
-/// searching the type declaration's array of all stored properties. If this
-/// index is not cached, it will cause widespread quadratic complexity in any
-/// pass that queries projections, including the SIL verifier.
-///
-/// FIXME: This cache may not be necessary if the Decl TypeChecker instead
-/// caches a field index in the VarDecl itself. This solution would be superior
-/// because it would allow constant time lookup of either the VarDecl or the
-/// index from a single pointer without referring back to a projection
-/// instruction.
-template <typename ParentTy>
-class FieldIndexCacheBase : public ParentTy {
-  enum : unsigned { InvalidFieldIndex = ~unsigned(0) };
-
-  VarDecl *field;
-
-public:
-  template <typename... ArgTys>
-  FieldIndexCacheBase(SILInstructionKind kind, SILDebugLocation loc,
-                      SILType type, VarDecl *field, ArgTys &&... extraArgs)
-      : ParentTy(kind, loc, type, std::forward<ArgTys>(extraArgs)...),
-        field(field) {
-    SILNode::Bits.FieldIndexCacheBase.FieldIndex = InvalidFieldIndex;
-    // This needs to be a concrete class to hold bitfield information. However,
-    // it should only be extended by UnaryInstructions.
-    assert(ParentTy::getNumOperands() == 1);
-  }
-
-  VarDecl *getField() const { return field; }
-
-  unsigned getFieldIndex() const {
-    unsigned idx = SILNode::Bits.FieldIndexCacheBase.FieldIndex;
-    if (idx != InvalidFieldIndex)
-      return idx;
-
-    return const_cast<FieldIndexCacheBase *>(this)->cacheFieldIndex();
-  }
-
-  NominalTypeDecl *getParentDecl() const {
-    auto s =
-        ParentTy::getOperand(0)->getType().getNominalOrBoundGenericNominal();
-    assert(s);
-    return s;
-  }
-
-  static bool classof(SILNodePointer node) {
-    SILNodeKind kind = node->getKind();
-    return kind == SILNodeKind::StructExtractInst ||
-           kind == SILNodeKind::StructElementAddrInst ||
-           kind == SILNodeKind::RefElementAddrInst;
-  }
-
-private:
-  unsigned cacheFieldIndex() {
-    unsigned index = swift::getFieldIndex(getParentDecl(), getField());
-    SILNode::Bits.FieldIndexCacheBase.FieldIndex = index;
-    return index;
-  }
-};
-
 /// Extract a physical, fragile field out of a value of struct type.
 class StructExtractInst
     : public UnaryInstructionBase<
           SILInstructionKind::StructExtractInst,
-          FieldIndexCacheBase<GuaranteedFirstArgForwardingSingleValueInst>> {
+          GuaranteedFirstArgForwardingSingleValueInst> {
   friend SILBuilder;
 
+  VarDecl *field;
+
   StructExtractInst(SILDebugLocation DebugLoc, SILValue Operand, VarDecl *Field,
                     SILType ResultTy,
                     ValueOwnershipKind forwardingOwnershipKind)
-      : UnaryInstructionBase(DebugLoc, Operand, ResultTy, Field,
-                             forwardingOwnershipKind) {}
+      : UnaryInstructionBase(DebugLoc, Operand, ResultTy,
+                             forwardingOwnershipKind),
+        field(Field) {}
 
 public:
+  VarDecl *getField() const { return field; }
+
+  unsigned getFieldIndex() const { return getField()->getFieldIndex(); }
+
   StructDecl *getStructDecl() const {
-    return cast<StructDecl>(getParentDecl());
+    return getOperand()->getType().getStructOrBoundGenericStruct();
   }
 
   /// Returns true if this is a trivial result of a struct that is non-trivial
@@ -6363,34 +6303,48 @@ class StructExtractInst
 /// Derive the address of a physical field from the address of a struct.
 class StructElementAddrInst
     : public UnaryInstructionBase<SILInstructionKind::StructElementAddrInst,
-                                  FieldIndexCacheBase<SingleValueInstruction>> {
+                                  SingleValueInstruction> {
   friend SILBuilder;
 
+  VarDecl *field;
+
   StructElementAddrInst(SILDebugLocation DebugLoc, SILValue Operand,
                         VarDecl *Field, SILType ResultTy)
-      : UnaryInstructionBase(DebugLoc, Operand, ResultTy, Field) {}
+      : UnaryInstructionBase(DebugLoc, Operand, ResultTy), field(Field) {}
 
 public:
+  VarDecl *getField() const { return field; }
+
+  unsigned getFieldIndex() const { return getField()->getFieldIndex(); }
+
   StructDecl *getStructDecl() const {
-    return cast<StructDecl>(getParentDecl());
+    return getOperand()->getType().getStructOrBoundGenericStruct();
   }
 };
 
 /// RefElementAddrInst - Derive the address of a named element in a reference
 /// type instance.
 class RefElementAddrInst
     : public UnaryInstructionBase<SILInstructionKind::RefElementAddrInst,
-                                  FieldIndexCacheBase<SingleValueInstruction>> {
+                                  SingleValueInstruction> {
   friend SILBuilder;
 
+  VarDecl *field;
+
   RefElementAddrInst(SILDebugLocation DebugLoc, SILValue Operand,
                      VarDecl *Field, SILType ResultTy, bool IsImmutable)
-      : UnaryInstructionBase(DebugLoc, Operand, ResultTy, Field) {
+      : UnaryInstructionBase(DebugLoc, Operand, ResultTy), field(Field) {
     setImmutable(IsImmutable);
   }
 
 public:
-  ClassDecl *getClassDecl() const { return cast<ClassDecl>(getParentDecl()); }
+  VarDecl *getField() const { return field; }
+
+  unsigned getFieldIndex() const { return getField()->getFieldIndex(); }
+
+  ClassDecl *getClassDecl() const {
+    return getOperand()->getType().getClassOrBoundGenericClass();
+  }
 
   /// Returns true if all loads of the same instance variable from the same
   /// class reference operand are guaranteed to yield the same value.

include/swift/SIL/SILNode.h

@@ -348,12 +348,6 @@ class alignas(8) SILNode {
     KeepUnique : 1
   );
 
-  SWIFT_INLINE_BITFIELD_FULL_TEMPLATE(FieldIndexCacheBase,
-                                      SingleValueInstruction, 32,
-    : NumPadBits,
-    FieldIndex : 32
-  );
-
   SWIFT_INLINE_BITFIELD_EMPTY(MethodInst, SingleValueInstruction);
   // Ensure that WitnessMethodInst bitfield does not overflow.
   IBWTO_BITFIELD_EMPTY(WitnessMethodInst, MethodInst);

lib/AST/Decl.cpp

@@ -34,6 +34,7 @@
 #include "swift/AST/Module.h"
 #include "swift/AST/NameLookup.h"
 #include "swift/AST/NameLookupRequests.h"
+#include "swift/AST/SILOptimizerRequests.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/ParseRequests.h"
 #include "swift/AST/Pattern.h"
@@ -5518,6 +5519,16 @@ VarDecl::VarDecl(DeclKind kind, bool isStatic, VarDecl::Introducer introducer,
   Bits.VarDecl.IsTopLevelGlobal = false;
 }
 
+int VarDecl::getFieldIndex() const {
+  if (fieldIndex < 0) {
+    NominalTypeDecl *parent = getDeclContext()->getSelfNominalTypeDecl();
+    evaluateOrDefault(getASTContext().evaluator,
+                      ComputeFieldIndicesRequest{parent}, {});
+    assert(fieldIndex >= 0);
+  }
+  return fieldIndex;
+}
+
 Type VarDecl::getType() const {
   return getDeclContext()->mapTypeIntoContext(getInterfaceType());
 }

lib/SIL/IR/SILInstructions.cpp

@@ -1293,22 +1293,7 @@ bool TupleExtractInst::isEltOnlyNonTrivialElt() const {
 
 /// Get a unique index for a struct or class field in layout order.
 unsigned swift::getFieldIndex(NominalTypeDecl *decl, VarDecl *field) {
-  unsigned index = 0;
-  if (auto *classDecl = dyn_cast<ClassDecl>(decl)) {
-    for (auto *superDecl = classDecl->getSuperclassDecl(); superDecl != nullptr;
-         superDecl = superDecl->getSuperclassDecl()) {
-      index += superDecl->getStoredProperties().size();
-    }
-  }
-  for (VarDecl *property : decl->getStoredProperties()) {
-    if (field == property) {
-      return index;
-    }
-    ++index;
-  }
-  llvm_unreachable("The field decl for a struct_extract, struct_element_addr, "
-                   "or ref_element_addr must be an accessible stored "
-                   "property of the operand type");
+  return field->getFieldIndex();
 }
 
 /// Get the property for a struct or class by its unique index.

lib/SIL/Verifier/SILVerifier.cpp

@@ -2985,6 +2985,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
       requireSameType(loweredFieldTy, EI->getType(),
                       "result of struct_extract does not match type of field");
     }
+    require(EI->getFieldIndex() < sd->getStoredProperties().size(),
+        "invalid field index for struct_extract instruction");
   }
 
   void checkTupleElementAddrInst(TupleElementAddrInst *EI) {
@@ -3033,6 +3035,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
           loweredFieldTy, EI->getType(),
           "result of struct_element_addr does not match type of field");
     }
+    require(EI->getFieldIndex() < sd->getStoredProperties().size(),
+        "invalid field index for struct_element_addr instruction");
   }
 
   void checkRefElementAddrInst(RefElementAddrInst *EI) {
@@ -3060,7 +3064,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
           loweredFieldTy, EI->getType(),
           "result of ref_element_addr does not match type of field");
     }
-    EI->getFieldIndex();  // Make sure we can access the field without crashing.
+    require(EI->getFieldIndex() < cd->getStoredProperties().size(),
+            "invalid field index for ref_element_addr instruction");
   }
 
   void checkRefTailAddrInst(RefTailAddrInst *RTAI) {

lib/SILOptimizer/PassManager/SILOptimizerRequests.cpp

@@ -88,6 +88,22 @@ LoweredSILRequest::evaluate(Evaluator &evaluator,
   return silMod;
 }
 
+std::tuple<>
+ComputeFieldIndicesRequest::evaluate(Evaluator &evaluator, NominalTypeDecl *decl) const {
+  unsigned index = 0;
+  if (auto *classDecl = dyn_cast<ClassDecl>(decl)) {
+    for (auto *superDecl = classDecl->getSuperclassDecl(); superDecl != nullptr;
+         superDecl = superDecl->getSuperclassDecl()) {
+      index += superDecl->getStoredProperties().size();
+    }
+  }
+  for (VarDecl *property : decl->getStoredProperties()) {
+    assert(property->fieldIndex < 0);
+    property->fieldIndex = index++;
+  }
+  return {};
+}
+
 // Define request evaluation functions for each of the SILGen requests.
 static AbstractRequestFunction *silOptimizerRequestFunctions[] = {
 #define SWIFT_REQUEST(Zone, Name, Sig, Caching, LocOptions)                    \