Merge pull request #41557 from atrick/addrlower-update

Update and reimplement AddressLowering pass (for SIL opaque values).

Author: atrick

docs/SIL.rst

@@ -2193,6 +2193,26 @@ parts::
        return %1 : $Klass
      }
 
+Forwarding Address-Only Values
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Address-only values are potentially unmovable when borrowed. This
+means that they cannot be forwarded with guaranteed ownership unless
+the forwarded value has the same representation as in the original
+value and can reuse the same storage. Non-destructive projection is
+allowed, such as `struct_extract`. Aggregation, such as `struct`, and
+destructive disaggregation, such as `switch_enum` is not allowed. This
+is an invariant for OSSA with opaque SIL values for these reasons:
+
+1. To avoid implicit semantic copies. For move-only values, this allows
+complete diagnostics. And in general, it makes it impossible for SIL
+passes to "accidentally" create copies.
+
+2. To reuse borrowed storage. This allows the optimizer to share the same
+storage for multiple exclusive reads of the same variable, avoiding
+copies. It may also be necessary to support native Swift atomics, which
+will be unmovable-when-borrowed.
+
 Borrowed Object based Safe Interior Pointers
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

include/swift/AST/SILOptions.h

@@ -140,6 +140,9 @@ class SILOptions {
   /// If this is disabled we do not serialize in OSSA form when optimizing.
   bool EnableOSSAModules = false;
 
+  /// If set to true, compile with the SIL Opaque Values enabled.
+  bool EnableSILOpaqueValues = false;
+
   // The kind of function bodies to skip emitting.
   FunctionBodySkipping SkipFunctionBodies = FunctionBodySkipping::None;
 

include/swift/Basic/LangOptions.h

@@ -379,11 +379,6 @@ namespace swift {
     /// [TODO: Clang-type-plumbing] Turn on for feature rollout.
     bool UseClangFunctionTypes = false;
 
-    /// If set to true, compile with the SIL Opaque Values enabled.
-    /// This is for bootstrapping. It can't be in SILOptions because the
-    /// TypeChecker uses it to set resolve the ParameterConvention.
-    bool EnableSILOpaqueValues = false;
-
     /// If set to true, the diagnosis engine can assume the emitted diagnostics
     /// will be used in editor. This usually leads to more aggressive fixit.
     bool DiagnosticsEditorMode = false;

include/swift/Option/FrontendOptions.td

@@ -507,9 +507,6 @@ def disable_sil_ownership_verifier : Flag<["-"], "disable-sil-ownership-verifier
 def suppress_static_exclusivity_swap : Flag<["-"], "suppress-static-exclusivity-swap">,
   HelpText<"Suppress static violations of exclusive access with swap()">;
 
-def enable_sil_opaque_values : Flag<["-"], "enable-sil-opaque-values">,
-  HelpText<"Enable SIL Opaque Values">;
-  
 def enable_experimental_static_assert :
   Flag<["-"], "enable-experimental-static-assert">,
   HelpText<"Enable experimental #assert">;
@@ -1022,6 +1019,9 @@ def enable_ossa_modules : Flag<["-"], "enable-ossa-modules">,
   HelpText<"Always serialize SIL in ossa form. If this flag is not passed in, "
            "when optimizing ownership will be lowered before serializing SIL">;
 
+def enable_sil_opaque_values : Flag<["-"], "enable-sil-opaque-values">,
+  HelpText<"Enable SIL Opaque Values">;
+
 def new_driver_path
   : Separate<["-"], "new-driver-path">, MetaVarName<"<path>">,
   HelpText<"Path of the new driver to be used">;

include/swift/SIL/ApplySite.h

@@ -585,9 +585,9 @@ class FullApplySite : public ApplySite {
   }
 
   /// Get the SIL value that represents all of the given call's results. For a
-  /// single direct result, returns the result. For multiple results, returns a
-  /// fake tuple value. The tuple has no storage of its own. The real results
-  /// must be extracted from it.
+  /// single direct result, returns the actual result. For multiple results,
+  /// returns a pseudo-result tuple. The tuple has no storage of its own. The
+  /// real results must be extracted from it.
   ///
   /// For ApplyInst, returns the single-value instruction itself.
   ///
@@ -596,7 +596,7 @@ class FullApplySite : public ApplySite {
   /// For BeginApplyInst, returns an invalid value. For coroutines, there is no
   /// single value representing all results. Yielded values are generally
   /// handled differently since they have the convention of incoming arguments.
-  SILValue getPseudoResult() const {
+  SILValue getResult() const {
     switch (getKind()) {
     case FullApplySiteKind::ApplyInst:
       return SILValue(cast<ApplyInst>(getInstruction()));

include/swift/SIL/SILBuilder.h

@@ -266,11 +266,10 @@ class SILBuilder {
   void clearInsertionPoint() { BB = nullptr; }
 
   /// setInsertionPoint - Set the insertion point.
-  void setInsertionPoint(SILBasicBlock *BB, SILBasicBlock::iterator InsertPt) {
+  void setInsertionPoint(SILBasicBlock *BB, SILBasicBlock::iterator insertPt) {
     this->BB = BB;
-    this->InsertPt = InsertPt;
-    if (InsertPt == BB->end())
-      return;
+    this->InsertPt = insertPt;
+    assert(insertPt == BB->end() || insertPt->getParent() == BB);
   }
 
   /// setInsertionPoint - Set the insertion point to insert before the specified

include/swift/SIL/SILModule.h

@@ -307,6 +307,12 @@ class SILModule {
   /// The stage of processing this module is at.
   SILStage Stage;
 
+  /// True if SIL conventions force address-only to be passed by address.
+  ///
+  /// Used for bootstrapping the AddressLowering pass. This should eventually
+  /// be inferred from the SIL stage to be true only when Stage == Lowered.
+  bool loweredAddresses;
+
   /// The set of deserialization notification handlers.
   DeserializationNotificationHandlerSet deserializationNotificationHandlers;
 
@@ -806,6 +812,11 @@ class SILModule {
     Stage = s;
   }
 
+  /// True if SIL conventions force address-only to be passed by address.
+  bool useLoweredAddresses() const { return loweredAddresses; }
+
+  void setLoweredAddresses(bool val) { loweredAddresses = val; }
+
   llvm::IndexedInstrProfReader *getPGOReader() const { return PGOReader.get(); }
 
   void setPGOReader(std::unique_ptr<llvm::IndexedInstrProfReader> IPR) {
@@ -972,15 +983,13 @@ inline bool SILOptions::supportsLexicalLifetimes(const SILModule &mod) const {
     // entirely.
     return LexicalLifetimes != LexicalLifetimesOption::Off;
   case SILStage::Canonical:
+  case SILStage::Lowered:
     // In Canonical SIL, lexical markers are used to ensure that object
     // lifetimes do not get observably shortened from the end of a lexical
     // scope.  That behavior only occurs when lexical lifetimes is (fully)
     // enabled.  (When only diagnostic markers are enabled, the markers are
     // stripped as part of lowering from raw to canonical SIL.)
     return LexicalLifetimes == LexicalLifetimesOption::On;
-  case SILStage::Lowered:
-    // We do not support OSSA in Lowered SIL, so this is always false.
-    return false;
   }
 }
 

lib/Frontend/CompilerInvocation.cpp

@@ -770,7 +770,6 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
   Opts.EnableObjCInterop =
       Args.hasFlag(OPT_enable_objc_interop, OPT_disable_objc_interop,
                    Target.isOSDarwin());
-  Opts.EnableSILOpaqueValues |= Args.hasArg(OPT_enable_sil_opaque_values);
 
   Opts.VerifyAllSubstitutionMaps |= Args.hasArg(OPT_verify_all_substitution_maps);
 
@@ -1678,6 +1677,7 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
   Opts.EnableARCOptimizations &= !Args.hasArg(OPT_disable_arc_opts);
   Opts.EnableOSSAModules |= Args.hasArg(OPT_enable_ossa_modules);
   Opts.EnableOSSAOptimizations &= !Args.hasArg(OPT_disable_ossa_opts);
+  Opts.EnableSILOpaqueValues |= Args.hasArg(OPT_enable_sil_opaque_values);
   Opts.EnableSpeculativeDevirtualization |= Args.hasArg(OPT_enable_spec_devirt);
   Opts.EnableActorDataRaceChecks |= Args.hasFlag(
       OPT_enable_actor_data_race_checks,

lib/SIL/IR/SILModule.cpp

@@ -91,8 +91,8 @@ class SILModule::SerializationCallback final
 
 SILModule::SILModule(llvm::PointerUnion<FileUnit *, ModuleDecl *> context,
                      Lowering::TypeConverter &TC, const SILOptions &Options)
-    : Stage(SILStage::Raw), indexTrieRoot(new IndexTrieNode()),
-      Options(Options), serialized(false),
+    : Stage(SILStage::Raw), loweredAddresses(!Options.EnableSILOpaqueValues),
+      indexTrieRoot(new IndexTrieNode()), Options(Options), serialized(false),
       regDeserializationNotificationHandlerForNonTransparentFuncOME(false),
       regDeserializationNotificationHandlerForAllFuncOME(false),
       prespecializedFunctionDeclsImported(false), SerializeSILAction(),

lib/SIL/IR/SILType.cpp

@@ -506,10 +506,7 @@ SILResultInfo::getOwnershipKind(SILFunction &F,
 }
 
 SILModuleConventions::SILModuleConventions(SILModule &M)
-    : M(&M),
-      loweredAddresses(!M.getASTContext().LangOpts.EnableSILOpaqueValues
-                       || M.getStage() == SILStage::Lowered)
-{}
+    : M(&M), loweredAddresses(M.useLoweredAddresses()) {}
 
 bool SILModuleConventions::isReturnedIndirectlyInSIL(SILType type,
                                                      SILModule &M) {

lib/SIL/IR/TypeLowering.cpp

@@ -1605,10 +1605,6 @@ namespace {
   };
 
   /// Lower address only types as opaque values.
-  ///
-  /// Opaque values behave like loadable leaf types in SIL.
-  ///
-  /// FIXME: When you remove an unreachable, just delete the method.
   class OpaqueValueTypeLowering : public LeafLoadableTypeLowering {
   public:
     OpaqueValueTypeLowering(SILType type, RecursiveProperties properties,
@@ -1622,6 +1618,20 @@ namespace {
       llvm_unreachable("copy into");
     }
 
+    // OpaqueValue store cannot be decoupled from a destroy because it is not
+    // bitwise-movable.
+    void emitStore(SILBuilder &B, SILLocation loc, SILValue value,
+                   SILValue addr, StoreOwnershipQualifier qual) const override {
+      B.createStore(loc, value, addr, qual);
+    }
+
+    // OpaqueValue load cannot be decoupled from a copy because it is not
+    // bitwise-movable.
+    SILValue emitLoad(SILBuilder &B, SILLocation loc, SILValue addr,
+                      LoadOwnershipQualifier qual) const override {
+      return B.createLoad(loc, addr, qual);
+    }
+
     // --- Same as LeafLoadableTypeLowering.
 
     SILValue emitLoweredCopyValue(SILBuilder &B, SILLocation loc,
@@ -1683,7 +1693,7 @@ namespace {
 
     TypeLowering *handleAddressOnly(CanType type,
                                     RecursiveProperties properties) {
-      if (!TC.Context.LangOpts.EnableSILOpaqueValues) {
+      if (!TC.Context.SILOpts.EnableSILOpaqueValues) {
         auto silType = SILType::getPrimitiveAddressType(type);
         return new (TC) AddressOnlyTypeLowering(silType, properties,
                                                            Expansion);

lib/SIL/Parser/ParseSIL.cpp

@@ -6535,6 +6535,9 @@ bool SILParserState::parseDeclSILStage(Parser &P) {
   }
 
   M.setStage(stage);
+  if (M.getOptions().EnableSILOpaqueValues) {
+    M.setLoweredAddresses(stage != SILStage::Raw);
+  }
   DidParseSILStage = true;
   return false;
 }

lib/SIL/Verifier/SILVerifier.cpp

@@ -1008,6 +1008,13 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
       auto *TI = predBB->getTerminator();
       if (F.hasOwnership()) {
         require(isa<BranchInst>(TI), "All phi inputs must be branch operands.");
+
+        // Address-only values are potentially unmovable when borrowed. See also
+        // checkOwnershipForwardingInst. A phi implies a move of its arguments
+        // because they can't necessarilly all reuse the same storage.
+        require((!arg->getType().isAddressOnly(F)
+                 || arg->getOwnershipKind() != OwnershipKind::Guaranteed),
+                "Guaranteed address-only phi not allowed--implies a copy");
       } else {
         // FIXME: when critical edges are removed and cond_br arguments are
         // disallowed, only allow BranchInst.
@@ -1269,10 +1276,11 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
       checkOwnershipForwardingTermInst(term);
     }
 
-    // Address-only values are potentially move-only, and unmovable if they are
-    // borrowed. Ensure that guaranteed address-only values are forwarded with
-    // the same representation. Non-destructive projection is
-    // allowed. Aggregation and destructive disaggregation is not allowed.
+    // Address-only values are potentially unmovable when borrowed. Ensure that
+    // guaranteed address-only values are forwarded with the same
+    // representation. Non-destructive projection is allowed. Aggregation and
+    // destructive disaggregation is not allowed. See SIL.rst, Forwarding
+    // Addres-Only Values.
     if (ownership == OwnershipKind::Guaranteed
         && OwnershipForwardingMixin::isAddressOnly(i)) {
       require(OwnershipForwardingMixin::hasSameRepresentation(i),

lib/SILGen/SILGenFunction.cpp

@@ -290,13 +290,12 @@ void SILGenFunction::emitCaptures(SILLocation loc,
     // Get an address value for a SILValue if it is address only in an type
     // expansion context without opaque archetype substitution.
     auto getAddressValue = [&](SILValue entryValue) -> SILValue {
-      if (SGM.Types
-              .getTypeLowering(
-                  valueType,
-                  TypeExpansionContext::noOpaqueTypeArchetypesSubstitution(
-                      expansion.getResilienceExpansion()))
-              .isAddressOnly() &&
-          !entryValue->getType().isAddress()) {
+      if (SGM.Types.getTypeLowering(
+            valueType,
+            TypeExpansionContext::noOpaqueTypeArchetypesSubstitution(
+              expansion.getResilienceExpansion()))
+          .isAddressOnly()
+          && !entryValue->getType().isAddress()) {
 
         auto addr = emitTemporaryAllocation(vd, entryValue->getType());
         auto val = B.emitCopyValueOperation(vd, entryValue);

lib/SILGen/SILGenFunction.h

@@ -245,6 +245,8 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   /// The SILModuleConventions for this SIL module.
   SILModuleConventions silConv;
 
+  bool useLoweredAddresses() const { return silConv.useLoweredAddresses(); }
+
   /// The DeclContext corresponding to the function currently being emitted.
   DeclContext * const FunctionDC;