Add NestedSemanticFunctionCheck diagnostic

atrick · atrick · commit 3128eae3f0cd · 2020-10-26T17:02:33.000-07:00
to check for improperly nested '@_semantic' functions.

Add a missing @_semantics("array.init") in ArraySlice found by the
diagnostic.

Distinguish between array.init and array.init.empty.

Categorize the types of semantic functions by how they affect the
inliner and pass pipeline, and centralize this logic in
PerformanceInlinerUtils. The ultimate goal is to prevent inlining of
"Fundamental" @_semantics calls and @_effects calls until the late
pipeline where we can safely discard semantics. However, that requires
significant pipeline changes.

In the meantime, this change prevents the situation from getting worse
and makes the intention clear. However, it has no significant effect
on the pass pipeline and inliner.
diff --git a/include/swift/AST/DiagnosticsSIL.def b/include/swift/AST/DiagnosticsSIL.def
@@ -643,5 +643,7 @@ ERROR(box_to_stack_cannot_promote_box_to_stack_due_to_escape_alloc, none,
 NOTE(box_to_stack_cannot_promote_box_to_stack_due_to_escape_location, none,
      "value escapes here", ())
 
+WARNING(semantic_function_improper_nesting, none, "'@_semantics' function calls non-'@_semantics' function with nested '@_semantics' calls", ())
+
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"
diff --git a/include/swift/SILOptimizer/Analysis/ArraySemantic.h b/include/swift/SILOptimizer/Analysis/ArraySemantic.h
@@ -42,6 +42,7 @@ enum class ArrayCallKind {
   // a function, and it has a self parameter, make sure that it is defined
   // before this comment.
   kArrayInit,
+  kArrayInitEmpty,
   kArrayUninitialized,
   kArrayUninitializedIntrinsic,
   kArrayFinalizeIntrinsic
@@ -187,7 +188,7 @@ class ArraySemanticsCall {
 
   /// Could this array be backed by an NSArray.
   bool mayHaveBridgedObjectElementType() const;
-  
+
   /// Can this function be inlined by the early inliner.
   bool canInlineEarly() const;
 
diff --git a/include/swift/SILOptimizer/PassManager/Passes.def b/include/swift/SILOptimizer/PassManager/Passes.def
@@ -251,6 +251,8 @@ PASS(Outliner, "outliner",
      "Function Outlining Optimization")
 PASS(OwnershipModelEliminator, "ownership-model-eliminator",
      "Eliminate Ownership Annotation of SIL")
+PASS(NestedSemanticFunctionCheck, "nested-semantic-function-check",
+     "Diagnose improperly nested '@_semantics' functions")
 PASS(NonTransparentFunctionOwnershipModelEliminator,
      "non-transparent-func-ownership-model-eliminator",
      "Eliminate Ownership Annotations from non-transparent SIL Functions")
diff --git a/include/swift/SILOptimizer/Utils/PerformanceInlinerUtils.h b/include/swift/SILOptimizer/Utils/PerformanceInlinerUtils.h
@@ -39,13 +39,38 @@ enum class InlineSelection {
   OnlyInlineAlways,
 };
 
-// Returns the callee of an apply_inst if it is basically inlinable.
+/// Check if this ApplySite is eligible for inlining. If so, return the callee.
 SILFunction *getEligibleFunction(FullApplySite AI,
                                  InlineSelection WhatToInline);
 
 // Returns true if this is a pure call, i.e. the callee has no side-effects
 // and all arguments are constants.
 bool isPureCall(FullApplySite AI, SideEffectAnalysis *SEA);
+
+/// Fundamental @_semantic tags provide additional semantics for optimization
+/// beyond what SIL analysis can discover from the function body. They should be
+/// preserved until semantic passes can process them. For example, they may need
+/// to be hoisted by LICM after inlining up to the highest level caller.
+///
+/// Nested @_semantic tags also provide semantics beyond the function body but
+/// they hide the semantics of underlying calls. These need to be inlined into
+/// the highest level caller before semantics passes can process the underlying
+/// Fundamental semantic functions.
+///
+/// Transient @_semantic tags are not required for optimization.
+enum class SemanticFunctionLevel { Fundamental, Transient, Nested };
+
+/// Return the SemanticFunctionLevel of \p callee.
+SemanticFunctionLevel getSemanticFunctionLevel(SILFunction *function);
+
+inline bool isOptimizableSemanticFunction(SILFunction *function) {
+  return getSemanticFunctionLevel(function) != SemanticFunctionLevel::Transient;
+}
+
+/// Return true if \p apply calls into an optimizable semantic function from
+/// within another semantic function, or from a "trivial" wrapper.
+bool isNestedSemanticCall(FullApplySite apply);
+
 } // end swift namespace
 
 //===----------------------------------------------------------------------===//
diff --git a/lib/SILOptimizer/Analysis/ArraySemantic.cpp b/lib/SILOptimizer/Analysis/ArraySemantic.cpp
@@ -30,7 +30,8 @@ ArrayCallKind swift::getArraySemanticsKind(SILFunction *f) {
         llvm::StringSwitch<ArrayCallKind>(Attrs)
             .Case("array.props.isNativeTypeChecked",
                   ArrayCallKind::kArrayPropsIsNativeTypeChecked)
-            .StartsWith("array.init", ArrayCallKind::kArrayInit)
+            .Case("array.init", ArrayCallKind::kArrayInit)
+            .Case("array.init.empty", ArrayCallKind::kArrayInitEmpty)
             .Case("array.uninitialized", ArrayCallKind::kArrayUninitialized)
             .Case("array.uninitialized_intrinsic", ArrayCallKind::kArrayUninitializedIntrinsic)
             .Case("array.finalize_intrinsic", ArrayCallKind::kArrayFinalizeIntrinsic)
@@ -682,8 +683,10 @@ static SILValue getArrayUninitializedInitResult(ArraySemanticsCall arrayCall,
 
 SILValue swift::ArraySemanticsCall::getArrayValue() const {
   ArrayCallKind arrayCallKind = getKind();
-  if (arrayCallKind == ArrayCallKind::kArrayInit)
+  if (arrayCallKind == ArrayCallKind::kArrayInit
+      || arrayCallKind == ArrayCallKind::kArrayInitEmpty) {
     return SILValue(SemanticsCall);
+  }
   return getArrayUninitializedInitResult(*this, 0);
 }
 
diff --git a/lib/SILOptimizer/Analysis/EscapeAnalysis.cpp b/lib/SILOptimizer/Analysis/EscapeAnalysis.cpp
@@ -1943,6 +1943,7 @@ void EscapeAnalysis::analyzeInstruction(SILInstruction *I,
       !isa<BeginApplyInst>(I)) {
     ArraySemanticsCall ASC(FAS.getInstruction());
     switch (ASC.getKind()) {
+      // TODO: Model ReserveCapacityForAppend, AppendContentsOf, AppendElement.
       case ArrayCallKind::kArrayPropsIsNativeTypeChecked:
       case ArrayCallKind::kCheckSubscript:
       case ArrayCallKind::kCheckIndex:
diff --git a/lib/SILOptimizer/IPO/GlobalPropertyOpt.cpp b/lib/SILOptimizer/IPO/GlobalPropertyOpt.cpp
@@ -269,6 +269,7 @@ void GlobalPropertyOpt::scanInstruction(swift::SILInstruction *Inst) {
     ArraySemanticsCall semCall(AI);
     switch (semCall.getKind()) {
       case ArrayCallKind::kArrayInit:
+      case ArrayCallKind::kArrayInitEmpty:
       case ArrayCallKind::kArrayUninitialized:
       case ArrayCallKind::kMutateUnknown:
       case ArrayCallKind::kMakeMutable:
diff --git a/lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp b/lib/SILOptimizer/LoopTransforms/COWArrayOpt.cpp
@@ -295,6 +295,7 @@ static bool isNonMutatingArraySemanticCall(SILInstruction *Inst) {
   case ArrayCallKind::kReserveCapacityForAppend:
   case ArrayCallKind::kWithUnsafeMutableBufferPointer:
   case ArrayCallKind::kArrayInit:
+  case ArrayCallKind::kArrayInitEmpty:
   case ArrayCallKind::kArrayUninitialized:
   case ArrayCallKind::kArrayUninitializedIntrinsic:
   case ArrayCallKind::kArrayFinalizeIntrinsic:
@@ -658,6 +659,7 @@ bool COWArrayOpt::hasLoopOnlyDestructorSafeArrayOperations() {
         auto Kind = Sem.getKind();
         // Safe because they create new arrays.
         if (Kind == ArrayCallKind::kArrayInit ||
+            Kind == ArrayCallKind::kArrayInitEmpty ||
             Kind == ArrayCallKind::kArrayUninitialized ||
             Kind == ArrayCallKind::kArrayUninitializedIntrinsic)
           continue;
diff --git a/lib/SILOptimizer/LoopTransforms/LoopUnroll.cpp b/lib/SILOptimizer/LoopTransforms/LoopUnroll.cpp
@@ -194,7 +194,7 @@ static bool canAndShouldUnrollLoop(SILLoop *Loop, uint64_t TripCount) {
         auto Callee = AI.getCalleeFunction();
         if (Callee && getEligibleFunction(AI, InlineSelection::Everything)) {
           // If callee is rather big and potentialy inlinable, it may be better
-          // not to unroll, so that the body of the calle can be inlined later.
+          // not to unroll, so that the body of the callee can be inlined later.
           Cost += Callee->size() * InsnsPerBB;
         }
       }
diff --git a/lib/SILOptimizer/Mandatory/CMakeLists.txt b/lib/SILOptimizer/Mandatory/CMakeLists.txt
@@ -14,6 +14,7 @@ target_sources(swiftSILOptimizer PRIVATE
   Differentiation.cpp
   IRGenPrepare.cpp
   MandatoryInlining.cpp
+  NestedSemanticFunctionCheck.cpp
   PredictableMemOpt.cpp
   PMOMemoryUseCollector.cpp
   RawSILInstLowering.cpp
diff --git a/lib/SILOptimizer/Mandatory/NestedSemanticFunctionCheck.cpp b/lib/SILOptimizer/Mandatory/NestedSemanticFunctionCheck.cpp
@@ -0,0 +1,142 @@
+//===--- NestedSemanticFunctionCheck.cpp - Disallow nested @_semantic -----===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2020 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 pass raises a diagnostic error if any semantic functions in the current
+/// module are improperly nested. A semantic function has an @_semantic
+/// attribute. Semantic functions may call other semantic functions, but
+/// semantic and non-semantic call frames may not be interleaved.
+///
+/// @_semantics(...)
+/// func funcA() {
+///   funcB()
+/// }
+/// // Error: funcB is called by a semantic function and calls
+/// // a semantic function.
+/// func funcB() {
+///   funcC()
+/// }
+/// @_semantics(...)
+/// func funcC() {}
+///
+/// For the pass pipeline to function well, @_semantic calls need to be
+/// processed top-down, while inlining proceeds bottom-up. With proper nesting,
+/// this is easily accomplished, but with improper nesting, semantic function
+/// can be prematurely "hidden".
+///
+/// For simplicity, don't bother checking whether semantic functions in one
+/// module call non-semantic functions in a different module. Really,
+/// "optimizable" @_semantics should only exist in the standard library.
+///
+/// TODO: The @_semantics tag should only ever be used to convey function
+/// semantics that are important to optimizations and which cannot be determined
+/// from the function body. Make sure this tag is never used for compiler hints
+/// or informational attributes, which do not require special pass pipeline and
+/// module serialization behavior.
+///
+//===----------------------------------------------------------------------===//
+
+#include "swift/AST/DiagnosticsSIL.h"
+#include "swift/SILOptimizer/PassManager/Transforms.h"
+#include "swift/SILOptimizer/Utils/PerformanceInlinerUtils.h"
+
+using namespace swift;
+
+namespace {
+
+class NestedSemanticFunctionCheck : public SILFunctionTransform {
+  // Mark all functions that may indirectly call a semantic function, ignoring
+  // simple wrappers, but are not themselves an optimizable semantic function.
+  //
+  // This could be a very large set. It is built gradually during bottom-up
+  // function transforms, then deleted once all functions are processed before
+  // excuting the next pass pipeline.
+  llvm::SmallPtrSet<SILFunction *, 8> mayCallSemanticFunctions;
+
+  // Mark semantic functions and wrappers around semantic calls.
+  llvm::SmallPtrSet<SILFunction *, 8> semanticFunctions;
+
+public:
+  void run() override;
+
+protected:
+  void checkSemanticFunction(SILFunction *f);
+};
+
+} // end anonymous namespace
+
+/// This is a semantic function. Diagnose any calls to mayCallSemanticFunctions.
+void NestedSemanticFunctionCheck::checkSemanticFunction(SILFunction *f) {
+  for (auto &bb : *f) {
+    for (auto &i : bb) {
+      auto apply = FullApplySite::isa(&i);
+      if (!apply) {
+        continue;
+      }
+      auto callee = apply.getReferencedFunctionOrNull();
+      if (!callee) {
+        continue;
+      }
+      // Semantic function calling non-semantic function with improperly
+      // nested semantic calls underneath.
+      if (mayCallSemanticFunctions.count(callee)) {
+        ASTContext &astContext = f->getASTContext();
+        astContext.Diags.diagnose(apply.getLoc().getSourceLoc(),
+                                  diag::semantic_function_improper_nesting);
+      }
+    }
+  }
+}
+
+// If this is a semantic function, diagnose it immediately. Otherwise propagate
+// the mayCallSemanticFunctions and semanticCallWrappers sets upward in the call
+// tree.
+void NestedSemanticFunctionCheck::run() {
+  auto *f = getFunction();
+  if (!semanticFunctions.count(f) && isOptimizableSemanticFunction(f)) {
+    semanticFunctions.insert(f);
+    checkSemanticFunction(f);
+    return;
+  }
+  // Add this function to mayCallSemanticFunctions if needed.
+  if (mayCallSemanticFunctions.count(f)) {
+    return;
+  }
+  for (auto &bb : *f) {
+    for (auto &i : bb) {
+      auto apply = FullApplySite::isa(&i);
+      if (!apply) {
+        continue;
+      }
+      // If this is a trivial wrapper around a semantic call, don't add it
+      // immediately to the mayCallSemanticFunctions set, but add it to
+      // semanticCallWrappers so that its caller will be added to
+      // mayCallSemanticFunctions.
+      if (isNestedSemanticCall(apply)) {
+        semanticFunctions.insert(f);
+        continue;
+      }
+      auto callee = apply.getReferencedFunctionOrNull();
+      if (!callee) {
+        continue;
+      }
+      // Propagate mayCallSemanticFunctions.
+      if (mayCallSemanticFunctions.count(callee)
+          || semanticFunctions.count(callee)) {
+        mayCallSemanticFunctions.insert(f);
+      }
+    }
+  }
+}
+
+SILTransform *swift::createNestedSemanticFunctionCheck() {
+  return new NestedSemanticFunctionCheck();
+}
diff --git a/lib/SILOptimizer/PassManager/PassPipeline.cpp b/lib/SILOptimizer/PassManager/PassPipeline.cpp
@@ -90,6 +90,7 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
   P.addSILGenCleanup();
   P.addDiagnoseInvalidEscapingCaptures();
   P.addDiagnoseStaticExclusivity();
+  P.addNestedSemanticFunctionCheck();
   P.addCapturePromotion();
 
   // Select access kind after capture promotion and before stack promotion.
diff --git a/lib/SILOptimizer/Utils/PerformanceInlinerUtils.cpp b/lib/SILOptimizer/Utils/PerformanceInlinerUtils.cpp
diff --git a/stdlib/public/core/ArraySlice.swift b/stdlib/public/core/ArraySlice.swift
diff --git a/stdlib/public/core/ContiguousArray.swift b/stdlib/public/core/ContiguousArray.swift
diff --git a/test/SILOptimizer/diagnose_nested_semantic.swift b/test/SILOptimizer/diagnose_nested_semantic.swift
diff --git a/test/SILOptimizer/inline_semantics.sil b/test/SILOptimizer/inline_semantics.sil

Original file line number	Diff line number	Diff line change
`@@ -194,7 +194,7 @@ static bool canAndShouldUnrollLoop(SILLoop *Loop, uint64_t TripCount) {`
`194`	`194`	`auto Callee = AI.getCalleeFunction();`
`195`	`195`	`if (Callee && getEligibleFunction(AI, InlineSelection::Everything)) {`
`196`	`196`	`// If callee is rather big and potentialy inlinable, it may be better`
`197`		`- // not to unroll, so that the body of the calle can be inlined later.`
	`197`	`+ // not to unroll, so that the body of the callee can be inlined later.`
`198`	`198`	`Cost += Callee->size() * InsnsPerBB;`
`199`	`199`	`}`
`200`	`200`	`}`