Merge pull request #35872 from gottesmm/ossa-serialize-then-lower-ownership

Author: swift-ci

include/swift/AST/SILOptions.h

@@ -153,6 +153,11 @@ class SILOptions {
   /// Emit extra exclusvity markers for memory access and verify coverage.
   bool VerifyExclusivity = false;
 
+  /// When building the stdlib with opts should we lower ownership after
+  /// serialization? Otherwise we do before. Only set to true on Darwin for now.
+  ///
+  bool SerializeStdlibWithOwnershipWithOpts = false;
+
   /// Calls to the replaced method inside of the replacement method will call
   /// the previous implementation.
   ///

include/swift/SIL/SILArgument.h

@@ -215,6 +215,12 @@ class SILPhiArgument : public SILArgument {
   /// this will be guaranteed to return a valid SILValue.
   SILValue getIncomingPhiValue(SILBasicBlock *predBlock) const;
 
+  /// If this argument is a true phi, return the operand in the \p predBLock
+  /// associated with an incoming value.
+  ///
+  /// \returns the operand or nullptr if this is not a true phi.
+  Operand *getIncomingPhiOperand(SILBasicBlock *predBlock) const;
+
   /// If this argument is a phi, populate `OutArray` with the incoming phi
   /// values for each predecessor BB. If this argument is not a phi, return
   /// false.

lib/Frontend/CompilerInvocation.cpp

@@ -1204,6 +1204,11 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
       Args.hasArg(OPT_sil_stop_optzns_before_lowering_ownership);
   if (const Arg *A = Args.getLastArg(OPT_external_pass_pipeline_filename))
     Opts.ExternalPassPipelineFilename = A->getValue();
+  // If our triple is a darwin triple, lower ownership on the stdlib after we
+  // serialize.
+  if (Triple.isOSDarwin()) {
+    Opts.SerializeStdlibWithOwnershipWithOpts = true;
+  }
 
   Opts.GenerateProfile |= Args.hasArg(OPT_profile_generate);
   const Arg *ProfileUse = Args.getLastArg(OPT_profile_use);

lib/IRGen/IRGenSIL.cpp

@@ -15,8 +15,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/IR/Constant.h"
 #define DEBUG_TYPE "irgensil"
+
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/IRGenOptions.h"
 #include "swift/AST/ParameterList.h"
@@ -47,6 +47,8 @@
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/TinyPtrVector.h"
+#include "llvm/IR/Constant.h"
+#include "llvm/IR/Constants.h"
 #include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/InlineAsm.h"
@@ -700,6 +702,10 @@ class IRGenSILFunction :
   ///
   /// - CodeGen Prepare may drop dbg.values pointing to PHI instruction.
   bool needsShadowCopy(llvm::Value *Storage) {
+    // If we have a constant data vector, we always need a shadow copy due to
+    // bugs in LLVM.
+    if (isa<llvm::ConstantDataVector>(Storage))
+      return true;
     return !isa<llvm::Constant>(Storage);
   }
 

lib/SIL/IR/SILArgument.cpp

@@ -161,6 +161,12 @@ bool SILPhiArgument::getIncomingPhiValues(
   return true;
 }
 
+Operand *SILPhiArgument::getIncomingPhiOperand(SILBasicBlock *predBlock) const {
+  if (!isPhiArgument())
+    return nullptr;
+  return getIncomingPhiOperandForPred(getParent(), predBlock, getIndex());
+}
+
 bool SILPhiArgument::getIncomingPhiOperands(
     SmallVectorImpl<Operand *> &returnedPhiOperands) const {
   if (!isPhiArgument())

lib/SIL/Verifier/MemoryLifetime.cpp

@@ -142,6 +142,8 @@ static bool canHandleAllocStack(AllocStackInst *asi) {
   if (asi->hasDynamicLifetime())
     return false;
 
+  SILType stackType = asi->getType();
+
   // Currently in this verifier, we stop verifying if we find a switch_enum_addr
   // use. This creates a problem since no one has gone through and changed the
   // frontend/optimizer to understand that it needs to insert destroy_addr on
@@ -155,9 +157,16 @@ static bool canHandleAllocStack(AllocStackInst *asi) {
   // implemented.
   //
   // https://bugs.swift.org/browse/SR-14123
-  if (asi->getType().getEnumOrBoundGenericEnum())
+  if (stackType.getEnumOrBoundGenericEnum())
     return false;
 
+  // Same for tuples that have an enum element. We are just working around this
+  // for now until the radar above is solved.
+  if (auto tt = stackType.getAs<TupleType>())
+    for (unsigned i : range(tt->getNumElements()))
+      if (stackType.getTupleElementType(i).getEnumOrBoundGenericEnum())
+        return false;
+
   // Otherwise we can optimize!
   return true;
 }

lib/SIL/Verifier/SILVerifier.cpp

@@ -682,6 +682,31 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
   LoadBorrowImmutabilityAnalysis loadBorrowImmutabilityAnalysis;
   bool SingleFunction = true;
 
+  /// A cache of the isOperandInValueUse check. When we process an operand, we
+  /// fix this for each of its uses.
+  llvm::DenseSet<std::pair<SILValue, const Operand *>> isOperandInValueUsesCache;
+
+  /// Check that this operand appears in the use-chain of the value it uses.
+  bool isOperandInValueUses(const Operand *operand) {
+    SILValue value = operand->get();
+
+    // First check the cache.
+    if (isOperandInValueUsesCache.contains({value, operand}))
+      return true;
+
+    // Otherwise, compute the value and initialize the cache for each of the
+    // operand's value uses.
+    bool foundUse = false;
+    for (auto *use : value->getUses()) {
+      if (use == operand) {
+        foundUse = true;
+      }
+      isOperandInValueUsesCache.insert({value, use});
+    }
+
+    return foundUse;
+  }
+
   SILVerifier(const SILVerifier&) = delete;
   void operator=(const SILVerifier&) = delete;
 public:
@@ -1112,7 +1137,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
 
       require(operand.getUser() == I,
               "instruction's operand's owner isn't the instruction");
-      require(isInValueUses(&operand), "operand value isn't used by operand");
+      require(isOperandInValueUses(&operand), "operand value isn't used by operand");
 
       if (operand.isTypeDependent()) {
         require(isa<SILInstruction>(I),
@@ -1311,14 +1336,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     });
   }
 
-  /// Check that this operand appears in the use-chain of the value it uses.
-  static bool isInValueUses(const Operand *operand) {
-    for (auto use : operand->get()->getUses())
-      if (use == operand)
-        return true;
-    return false;
-  }
-
   /// \return True if all of the users of the AllocStack instruction \p ASI are
   /// inside the same basic block.
   static bool isSingleBlockUsage(AllocStackInst *ASI, DominanceInfo *Dominance){

lib/SILOptimizer/LoopTransforms/LoopRotate.cpp

@@ -125,7 +125,9 @@ static void mapOperands(SILInstruction *inst,
 static void updateSSAForUseOfValue(
     SILSSAUpdater &updater, SmallVectorImpl<SILPhiArgument *> &insertedPhis,
     const llvm::DenseMap<ValueBase *, SILValue> &valueMap,
-    SILBasicBlock *Header, SILBasicBlock *EntryCheckBlock, SILValue Res) {
+    SILBasicBlock *Header, SILBasicBlock *EntryCheckBlock, SILValue Res,
+    SmallVectorImpl<std::pair<SILBasicBlock *, unsigned>>
+        &accumulatedAddressPhis) {
   // Find the mapped instruction.
   assert(valueMap.count(Res) && "Expected to find value in map!");
   SILValue MappedValue = valueMap.find(Res)->second;
@@ -159,49 +161,96 @@ static void updateSSAForUseOfValue(
            && "The entry check block should dominate the header");
     updater.rewriteUse(*use);
   }
-  // Canonicalize inserted phis to avoid extra BB Args.
+
+  // Canonicalize inserted phis to avoid extra BB Args and if we find an address
+  // phi, stash it so we can handle it after we are done rewriting.
+  bool hasOwnership = Header->getParent()->hasOwnership();
   for (SILPhiArgument *arg : insertedPhis) {
     if (SILValue inst = replaceBBArgWithCast(arg)) {
       arg->replaceAllUsesWith(inst);
       // DCE+SimplifyCFG runs as a post-pass cleanup.
       // DCE replaces dead arg values with undef.
       // SimplifyCFG deletes the dead BB arg.
+      continue;
     }
+
+    // If we didn't simplify and have an address phi, stash the value so we can
+    // fix it up.
+    if (hasOwnership && arg->getType().isAddress())
+      accumulatedAddressPhis.emplace_back(arg->getParent(), arg->getIndex());
   }
 }
 
-static void
-updateSSAForUseOfInst(SILSSAUpdater &updater,
-                      SmallVectorImpl<SILPhiArgument *> &insertedPhis,
-                      const llvm::DenseMap<ValueBase *, SILValue> &valueMap,
-                      SILBasicBlock *header, SILBasicBlock *entryCheckBlock,
-                      SILInstruction *inst) {
+static void updateSSAForUseOfInst(
+    SILSSAUpdater &updater, SmallVectorImpl<SILPhiArgument *> &insertedPhis,
+    const llvm::DenseMap<ValueBase *, SILValue> &valueMap,
+    SILBasicBlock *header, SILBasicBlock *entryCheckBlock, SILInstruction *inst,
+    SmallVectorImpl<std::pair<SILBasicBlock *, unsigned>>
+        &accumulatedAddressPhis) {
   for (auto result : inst->getResults())
     updateSSAForUseOfValue(updater, insertedPhis, valueMap, header,
-                           entryCheckBlock, result);
+                           entryCheckBlock, result, accumulatedAddressPhis);
 }
 
 /// Rewrite the code we just created in the preheader and update SSA form.
 static void rewriteNewLoopEntryCheckBlock(
     SILBasicBlock *header, SILBasicBlock *entryCheckBlock,
     const llvm::DenseMap<ValueBase *, SILValue> &valueMap) {
-  SmallVector<SILPhiArgument *, 4> insertedPhis;
+  SmallVector<std::pair<SILBasicBlock *, unsigned>, 8> accumulatedAddressPhis;
+  SmallVector<SILPhiArgument *, 8> insertedPhis;
   SILSSAUpdater updater(&insertedPhis);
 
-  // Fix PHIs (incoming arguments).
-  for (auto *arg : header->getArguments())
+  // Fix PHIs (incoming arguments). We iterate by index in case we replace the
+  // phi argument so we do not invalidate iterators.
+  for (unsigned i : range(header->getNumArguments())) {
+    auto *arg = header->getArguments()[i];
     updateSSAForUseOfValue(updater, insertedPhis, valueMap, header,
-                           entryCheckBlock, arg);
+                           entryCheckBlock, arg, accumulatedAddressPhis);
+  }
 
   auto instIter = header->begin();
 
   // The terminator might change from under us.
   while (instIter != header->end()) {
     auto &inst = *instIter;
     updateSSAForUseOfInst(updater, insertedPhis, valueMap, header,
-                          entryCheckBlock, &inst);
+                          entryCheckBlock, &inst, accumulatedAddressPhis);
     ++instIter;
   }
+
+  // Then see if any of our phis were address phis. In such a case, rewrite the
+  // address to be a smuggled through raw pointer. We do this late to
+  // conservatively not interfere with the previous code's invariants.
+  //
+  // We also translate the phis into a BasicBlock, Index form so we are careful
+  // with invalidation issues around branches/args.
+  auto rawPointerTy =
+      SILType::getRawPointerType(header->getParent()->getASTContext());
+  auto rawPointerUndef = SILUndef::get(rawPointerTy, header->getModule());
+  auto loc = RegularLocation::getAutoGeneratedLocation();
+  while (!accumulatedAddressPhis.empty()) {
+    SILBasicBlock *block;
+    unsigned argIndex;
+    std::tie(block, argIndex) = accumulatedAddressPhis.pop_back_val();
+    auto *arg = cast<SILPhiArgument>(block->getArgument(argIndex));
+    assert(arg->getType().isAddress() && "Not an address phi?!");
+    for (auto *predBlock : block->getPredecessorBlocks()) {
+      Operand *predUse = arg->getIncomingPhiOperand(predBlock);
+      SILBuilderWithScope builder(predUse->getUser());
+      auto *newIncomingValue =
+          builder.createAddressToPointer(loc, predUse->get(), rawPointerTy);
+      predUse->set(newIncomingValue);
+    }
+    SILBuilderWithScope builder(arg->getNextInstruction());
+    SILType oldArgType = arg->getType();
+    auto *phiShim = builder.createPointerToAddress(
+        loc, rawPointerUndef, oldArgType, true /*isStrict*/,
+        false /*is invariant*/);
+    arg->replaceAllUsesWith(phiShim);
+    SILArgument *newArg = block->replacePhiArgument(
+        argIndex, rawPointerTy, OwnershipKind::None, nullptr);
+    phiShim->setOperand(newArg);
+  }
 }
 
 /// Update the dominator tree after rotating the loop.