Merge pull request #29946 from apple/revert-28799-master

Revert "[SILOptimizer] Generalize optimization of static keypaths"

Author: jckarter

include/swift/SILOptimizer/Utils/KeyPathProjector.h

@@ -1460,7 +1460,6 @@ void SILGenModule::tryEmitPropertyDescriptor(AbstractStorageDecl *decl) {
                                                baseOperand, needsGenericContext,
                                                subs, decl, {},
                                                baseTy->getCanonicalType(),
-                                               M.getSwiftModule(),
                                                /*property descriptor*/ true);
 
   (void)SILProperty::create(M, /*serialized*/ false, decl, component);

lib/SILGen/SILGen.cpp

@@ -339,7 +339,6 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
                               AbstractStorageDecl *storage,
                               ArrayRef<ProtocolConformanceRef> indexHashables,
                               CanType baseTy,
-                              DeclContext *useDC,
                               bool forPropertyDescriptor);
 
   /// Known functions for bridging.

lib/SILGen/SILGen.h

@@ -3401,7 +3401,6 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
                                 AbstractStorageDecl *storage,
                                 ArrayRef<ProtocolConformanceRef> indexHashables,
                                 CanType baseTy,
-                                DeclContext *useDC,
                                 bool forPropertyDescriptor) {
   auto baseDecl = storage;
 
@@ -3471,8 +3470,8 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
     // supply the settability if needed. We only reference it here if the
     // setter is public.
     if (shouldUseExternalKeyPathComponent())
-      return storage->isSettable(useDC)
-        && storage->isSetterAccessibleFrom(useDC);
+      return storage->isSettable(M.getSwiftModule())
+        && storage->isSetterAccessibleFrom(M.getSwiftModule());
     return storage->isSettable(storage->getDeclContext());
   };
 
@@ -3626,7 +3625,6 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {
                             decl,
                             component.getSubscriptIndexHashableConformances(),
                             baseTy,
-                            SGF.FunctionDC,
                             /*for descriptor*/ false));
       baseTy = loweredComponents.back().getComponentType();
       if (kind == KeyPathExpr::Component::Kind::Property)

lib/SILGen/SILGenExpr.cpp

@@ -28,13 +28,11 @@
 #include "swift/SILOptimizer/Analysis/ValueTracking.h"
 #include "swift/SILOptimizer/Utils/CFGOptUtils.h"
 #include "swift/SILOptimizer/Utils/Existential.h"
-#include "swift/SILOptimizer/Utils/KeyPathProjector.h"
 #include "swift/SILOptimizer/Utils/ValueLifetime.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
-#include <utility>
 
 using namespace swift;
 using namespace swift::PatternMatch;
@@ -209,6 +207,92 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI,
   return NAI;
 }
 
+/// Ends the begin_access "scope" if a begin_access was inserted for optimizing
+/// a keypath pattern.
+static void insertEndAccess(BeginAccessInst *&beginAccess, bool isModify,
+                            SILBuilder &builder) {
+  if (beginAccess) {
+    builder.createEndAccess(beginAccess->getLoc(), beginAccess,
+                            /*aborted*/ false);
+    if (isModify)
+      beginAccess->setAccessKind(SILAccessKind::Modify);
+    beginAccess = nullptr;
+  }
+}
+
+/// Creates the projection pattern for a keypath instruction.
+///
+/// Currently only the StoredProperty pattern is handled.
+/// TODO: handle other patterns, like getters/setters, optional chaining, etc.
+///
+/// Returns false if \p keyPath is not a keypath instruction or if there is any
+/// other reason why the optimization cannot be done.
+static SILValue createKeypathProjections(SILValue keyPath, SILValue root,
+                                         SILLocation loc,
+                                         BeginAccessInst *&beginAccess,
+                                         SILBuilder &builder) {
+  if (auto *upCast = dyn_cast<UpcastInst>(keyPath))
+    keyPath = upCast->getOperand();
+
+  // Is it a keypath instruction at all?
+  auto *kpInst = dyn_cast<KeyPathInst>(keyPath);
+  if (!kpInst || !kpInst->hasPattern())
+    return SILValue();
+
+  auto components = kpInst->getPattern()->getComponents();
+
+  // Check if the keypath only contains patterns which we support.
+  for (const KeyPathPatternComponent &comp : components) {
+    if (comp.getKind() != KeyPathPatternComponent::Kind::StoredProperty)
+      return SILValue();
+  }
+
+  SILValue addr = root;
+  for (const KeyPathPatternComponent &comp : components) {
+    assert(comp.getKind() == KeyPathPatternComponent::Kind::StoredProperty);
+    VarDecl *storedProperty = comp.getStoredPropertyDecl();
+    SILValue elementAddr;
+    if (addr->getType().getStructOrBoundGenericStruct()) {
+      addr = builder.createStructElementAddr(loc, addr, storedProperty);
+    } else if (addr->getType().getClassOrBoundGenericClass()) {
+      SingleValueInstruction *Ref = builder.createLoad(loc, addr,
+                                         LoadOwnershipQualifier::Unqualified);
+      insertEndAccess(beginAccess, /*isModify*/ false, builder);
+
+      // Handle the case where the storedProperty is in a super class.
+      while (Ref->getType().getClassOrBoundGenericClass() !=
+             storedProperty->getDeclContext()) {
+        SILType superCl = Ref->getType().getSuperclass();
+        if (!superCl) {
+          // This should never happen, because the property should be in the
+          // decl or in a superclass of it. Just handle this to be on the safe
+          // side.
+          return SILValue();
+        }
+        Ref = builder.createUpcast(loc, Ref, superCl);
+      }
+
+      addr = builder.createRefElementAddr(loc, Ref, storedProperty);
+
+      // Class members need access enforcement.
+      if (builder.getModule().getOptions().EnforceExclusivityDynamic) {
+        beginAccess = builder.createBeginAccess(loc, addr, SILAccessKind::Read,
+                                                SILAccessEnforcement::Dynamic,
+                                                /*noNestedConflict*/ false,
+                                                /*fromBuiltin*/ false);
+        addr = beginAccess;
+      }
+    } else {
+      // This should never happen, as a stored-property pattern can only be
+      // applied to classes and structs. But to be safe - and future prove -
+      // let's handle this case and bail.
+      insertEndAccess(beginAccess, /*isModify*/ false, builder);
+      return SILValue();
+    }
+  }
+  return addr;
+}
+
 /// Try to optimize a keypath application with an apply instruction.
 ///
 /// Replaces (simplified SIL):
@@ -227,40 +311,36 @@ bool SILCombiner::tryOptimizeKeypath(ApplyInst *AI) {
     return false;
 
   SILValue keyPath, rootAddr, valueAddr;
-  bool isSet = false;
+  bool isModify = false;
   if (callee->getName() == "swift_setAtWritableKeyPath" ||
       callee->getName() == "swift_setAtReferenceWritableKeyPath") {
     keyPath = AI->getArgument(1);
     rootAddr = AI->getArgument(0);
     valueAddr = AI->getArgument(2);
-    isSet = true;
+    isModify = true;
   } else if (callee->getName() == "swift_getAtKeyPath") {
     keyPath = AI->getArgument(2);
     rootAddr = AI->getArgument(1);
     valueAddr = AI->getArgument(0);
   } else {
     return false;
   }
-  
-  auto projector = KeyPathProjector::create(keyPath, rootAddr,
-                                            AI->getLoc(), Builder);
-  if (!projector)
+
+  BeginAccessInst *beginAccess = nullptr;
+  SILValue projectedAddr = createKeypathProjections(keyPath, rootAddr,
+                                                    AI->getLoc(), beginAccess,
+                                                    Builder);
+  if (!projectedAddr)
     return false;
-  
-  KeyPathProjector::AccessType accessType;
-  if (isSet) accessType = KeyPathProjector::AccessType::Set;
-  else accessType = KeyPathProjector::AccessType::Get;
-  
-  projector->project(accessType, [&](SILValue projectedAddr) {
-    if (isSet) {
-      Builder.createCopyAddr(AI->getLoc(), valueAddr, projectedAddr,
-                             IsTake, IsInitialization);
-    } else {
-      Builder.createCopyAddr(AI->getLoc(), projectedAddr, valueAddr,
-                             IsNotTake, IsInitialization);
-    }
-  });
-  
+
+  if (isModify) {
+    Builder.createCopyAddr(AI->getLoc(), valueAddr, projectedAddr,
+                           IsTake, IsNotInitialization);
+  } else {
+    Builder.createCopyAddr(AI->getLoc(), projectedAddr, valueAddr,
+                           IsNotTake, IsInitialization);
+  }
+  insertEndAccess(beginAccess, isModify, Builder);
   eraseInstFromFunction(*AI);
   ++NumOptimizedKeypaths;
   return true;
@@ -305,24 +385,19 @@ bool SILCombiner::tryOptimizeInoutKeypath(BeginApplyInst *AI) {
   EndApplyInst *endApply = dyn_cast<EndApplyInst>(AIUse->getUser());
   if (!endApply)
     return false;
-  
-  auto projector = KeyPathProjector::create(keyPath, rootAddr,
-                                            AI->getLoc(), Builder);
-  if (!projector)
+
+  BeginAccessInst *beginAccess = nullptr;
+  SILValue projectedAddr = createKeypathProjections(keyPath, rootAddr,
+                                                    AI->getLoc(), beginAccess,
+                                                    Builder);
+  if (!projectedAddr)
     return false;
-    
-  KeyPathProjector::AccessType accessType;
-  if (isModify) accessType = KeyPathProjector::AccessType::Modify;
-  else accessType = KeyPathProjector::AccessType::Get;
-  
-  projector->project(accessType, [&](SILValue projectedAddr) {
-    // Replace the projected address.
-    valueAddr->replaceAllUsesWith(projectedAddr);
-    
-    // Skip to the end of the key path application before cleaning up.
-    Builder.setInsertionPoint(endApply);
-  });
 
+  // Replace the projected address.
+  valueAddr->replaceAllUsesWith(projectedAddr);
+
+  Builder.setInsertionPoint(endApply);
+  insertEndAccess(beginAccess, isModify, Builder);
   eraseInstFromFunction(*endApply);
   eraseInstFromFunction(*AI);
   ++NumOptimizedKeypaths;

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

@@ -11,7 +11,6 @@ silopt_register_sources(
   GenericCloner.cpp
   Generics.cpp
   InstOptUtils.cpp
-  KeyPathProjector.cpp
   LoadStoreOptUtils.cpp
   LoopUtils.cpp
   OptimizerStatsUtils.cpp