Merge pull request swiftlang#28635 from ravikandhadai/oslog-dead-code-elim-patch1

[SIL Optimization] Create new utilities for dead code elimination.

Author: ravikandhadai

include/swift/AST/Builtins.def

@@ -641,8 +641,10 @@ BUILTIN_MISC_OPERATION(PoundAssert, "poundAssert", "", Special)
 
 /// globalStringTablePointer has type String -> Builtin.RawPointer.
 /// It returns an immortal, global string table pointer for strings constructed
-/// from string literals.
-BUILTIN_MISC_OPERATION_WITH_SILGEN(GlobalStringTablePointer, "globalStringTablePointer", "", Special)
+/// from string literals. We consider it effects as readnone meaning that it
+/// does not read any memory (note that even though it reads from a string, it
+/// is a pure value and therefore we can consider it as readnone).
+BUILTIN_MISC_OPERATION_WITH_SILGEN(GlobalStringTablePointer, "globalStringTablePointer", "n", Special)
 
 #undef BUILTIN_MISC_OPERATION_WITH_SILGEN
 

include/swift/AST/DiagnosticsSIL.def

@@ -528,6 +528,9 @@ ERROR(oslog_non_constant_interpolation, none, "'OSLogInterpolation' instance "
 ERROR(oslog_property_not_constant, none, "'OSLogInterpolation.%0' is not a "
       "constant", (StringRef))
 
+ERROR(oslog_message_alive_after_opts, none, "OSLogMessage instance must not "
+      "be explicitly created and must be deletable", ())
+
 ERROR(global_string_pointer_on_non_constant, none, "globalStringTablePointer "
       "builtin must used only on string literals", ())
 

include/swift/SILOptimizer/Utils/ConstExpr.h

@@ -207,6 +207,8 @@ class ConstExprStepEvaluator {
   void dumpState();
 };
 
+bool hasConstantEvaluableAnnotation(SILFunction *fun);
+
 bool isConstantEvaluable(SILFunction *fun);
 
 /// Return true if and only if the given function \p fun is specially modeled

include/swift/SILOptimizer/Utils/InstOptUtils.h

@@ -56,8 +56,101 @@ NullablePtr<SILInstruction> createIncrementBefore(SILValue ptr,
 NullablePtr<SILInstruction> createDecrementBefore(SILValue ptr,
                                                   SILInstruction *insertpt);
 
+/// A utility for deleting one or more instructions belonging to a function, and
+/// cleaning up any dead code resulting from deleting those instructions. Use
+/// this utility instead of
+/// \c recursivelyDeleteTriviallyDeadInstruction.
+class InstructionDeleter {
+private:
+  // A set vector of instructions that are found to be dead. The ordering
+  // of instructions in this set is important as when a dead instruction is
+  // removed, new instructions will be generated to fix the lifetime of the
+  // instruction's operands. This has to be deterministic.
+  SmallSetVector<SILInstruction *, 8> deadInstructions;
+
+  void deleteInstruction(SILInstruction *inst,
+                         llvm::function_ref<void(SILInstruction *)> callback,
+                         bool fixOperandLifetimes);
+
+public:
+  InstructionDeleter() {}
+
+  /// If the instruction \p inst is dead, record it so that it can be cleaned
+  /// up.
+  void trackIfDead(SILInstruction *inst);
+
+  /// Delete the instruction \p inst and record instructions that may become
+  /// dead because of the removal of \c inst. This function will add necessary
+  /// ownership instructions to fix the lifetimes of the operands of \c inst to
+  /// compensate for its deletion. This function will not clean up dead code
+  /// resulting from the instruction's removal. To do so, invoke the method \c
+  /// cleanupDeadCode of this instance, once the SIL of the contaning function
+  /// is made consistent.
+  ///
+  /// \pre the function containing \c inst must be using ownership SIL.
+  /// \pre the instruction to be deleted must not have any use other than
+  /// incidental uses.
+  ///
+  /// \param callback a callback called whenever an instruction
+  /// is deleted.
+  void forceDeleteAndFixLifetimes(
+      SILInstruction *inst,
+      llvm::function_ref<void(SILInstruction *)> callback =
+          [](SILInstruction *) {});
+
+  /// Delete the instruction \p inst and record instructions that may become
+  /// dead because of the removal of \c inst. If in ownership SIL, use the
+  /// \c forceDeleteAndFixLifetimes function instead, unless under special
+  /// circumstances where the client must handle fixing lifetimes of the
+  /// operands of the deleted instructions. This function will not fix the
+  /// lifetimes of the operands of \c inst once it is deleted. This function
+  /// will not clean up dead code resulting from the instruction's removal. To
+  /// do so, invoke the method \c cleanupDeadCode of this instance, once the SIL
+  /// of the contaning function is made consistent.
+  ///
+  /// \pre the instruction to be deleted must not have any use other than
+  /// incidental uses.
+  ///
+  /// \param callback a callback called whenever an instruction
+  /// is deleted.
+  void forceDelete(
+      SILInstruction *inst,
+      llvm::function_ref<void(SILInstruction *)> callback =
+          [](SILInstruction *) {});
+
+  /// Clean up dead instructions that are tracked by this instance and all
+  /// instructions that transitively become dead.
+  ///
+  /// \pre the function contaning dead instructions must be consistent (i.e., no
+  /// under or over releases). Note that if \c forceDelete call leaves the
+  /// function body in an inconsistent state, it needs to be made consistent
+  /// before this method is invoked.
+  ///
+  /// \param callback a callback called whenever an instruction is deleted.
+  void
+  cleanUpDeadInstructions(llvm::function_ref<void(SILInstruction *)> callback =
+                              [](SILInstruction *) {});
+};
+
+/// If \c inst is dead, delete it and recursively eliminate all code that
+/// becomes dead because of that. If more than one instruction must
+/// be checked/deleted use the \c InstructionDeleter utility.
+///
+/// This function will add necessary compensation code to fix the lifetimes of
+/// the operands of the deleted instructions.
+///
+/// \pre the SIL function containing the instruction is assumed to be
+/// consistent, i.e., does not have under or over releases.
+///
+/// \param callback a callback called whenever an instruction is deleted.
+void eliminateDeadInstruction(
+    SILInstruction *inst, llvm::function_ref<void(SILInstruction *)> callback =
+                              [](SILInstruction *) {});
+
 /// For each of the given instructions, if they are dead delete them
-/// along with their dead operands.
+/// along with their dead operands. Note this utility must be phased out and
+/// replaced by \c eliminateDeadInstruction  and
+/// \c InstructionDeleter utilities.
 ///
 /// \param inst The ArrayRef of instructions to be deleted.
 /// \param force If Force is set, don't check if the top level instructions
@@ -69,7 +162,9 @@ void recursivelyDeleteTriviallyDeadInstructions(
     });
 
 /// If the given instruction is dead, delete it along with its dead
-/// operands.
+/// operands. Note this utility must be phased out and replaced by
+/// \c eliminateDeadInstruction and
+/// \c InstructionDeleter utilities.
 ///
 /// \param inst The instruction to be deleted.
 /// \param force If Force is set, don't check if the top level instruction is

lib/SILOptimizer/LoopTransforms/LICM.cpp

@@ -991,7 +991,7 @@ void LoopTreeOptimization::hoistLoadsAndStores(SILValue addr, SILLoop *loop, Ins
   }
 
   // In case the value is only stored but never loaded in the loop.
-  recursivelyDeleteTriviallyDeadInstructions(initialLoad);
+  eliminateDeadInstruction(initialLoad);
 }
 
 bool LoopTreeOptimization::hoistAllLoadsAndStores(SILLoop *loop) {

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

@@ -2010,8 +2010,13 @@ void LifetimeChecker::deleteDeadRelease(unsigned ReleaseID) {
   SILInstruction *Release = Destroys[ReleaseID];
   if (isa<DestroyAddrInst>(Release)) {
     SILValue Addr = Release->getOperand(0);
-    if (auto *AddrI = Addr->getDefiningInstruction())
+    if (auto *AddrI = Addr->getDefiningInstruction()) {
+      // FIXME: AddrI will not be deleted (nor its operands) when Release is
+      // still using AddrI's result. Fix this, and migrate to using
+      // InstructionDeleter utility instead of
+      // recursivelyDeadTriviallyDeadInstructions.
       recursivelyDeleteTriviallyDeadInstructions(AddrI);
+    }
   }
   Release->eraseFromParent();
   Destroys[ReleaseID] = nullptr;

lib/SILOptimizer/Mandatory/IRGenPrepare.cpp

@@ -56,12 +56,17 @@ static bool cleanFunction(SILFunction &fn) {
           LLVM_FALLTHROUGH;
         }
         case BuiltinValueKind::PoundAssert:
-        case BuiltinValueKind::StaticReport:
+        case BuiltinValueKind::StaticReport: {
           // The call to the builtin should get removed before we reach
           // IRGen.
-          recursivelyDeleteTriviallyDeadInstructions(bi, /* Force */ true);
+          InstructionDeleter deleter;
+          deleter.forceDelete(bi);
+          // StaticReport only takes trivial operands, and therefore doesn't
+          // require fixing the lifetime of its operands.
+          deleter.cleanUpDeadInstructions();
           madeChange = true;
           break;
+        }
         default:
           break;
       }

lib/SILOptimizer/Mandatory/OSLogOptimization.cpp

@@ -262,8 +262,7 @@ static bool shouldAttemptEvaluation(SILInstruction *inst) {
   SILFunction *calleeFun = apply->getCalleeFunction();
   if (!calleeFun)
     return false;
-  return isKnownConstantEvaluableFunction(calleeFun) ||
-         isConstantEvaluable(calleeFun);
+  return isConstantEvaluable(calleeFun);
 }
 
 /// Skip or evaluate the given instruction based on the evaluation policy and
@@ -780,20 +779,11 @@ static SILValue emitCodeForSymbolicValue(SymbolicValue symVal,
   }
 }
 
-/// Collect the end points of the instructions that are data dependent on \c
-/// value. A instruction is data dependent on \c value if its result may
-/// transitively depends on \c value. Note that data dependencies through
-/// addresses are not tracked by this function.
-///
-/// \param value SILValue that is not an address.
-/// \param fun SILFunction that defines \c value.
-/// \param endUsers buffer for storing the found end points of the data
-/// dependence chain.
-static void
-getEndPointsOfDataDependentChain(SILValue value, SILFunction *fun,
-                                 SmallVectorImpl<SILInstruction *> &endUsers) {
-  assert(!value->getType().isAddress());
-
+/// Given a SILValue \p value, compute the set of transitive users of the value
+/// (excluding value itself) by following the use-def chain starting at value.
+/// Note that this function does not follow use-def chains though branches.
+static void getTransitiveUsers(SILValue value,
+                               SmallVectorImpl<SILInstruction *> &users) {
   // Collect the instructions that are data dependent on the value using a
   // fix point iteration.
   SmallPtrSet<SILInstruction *, 16> visitedUsers;
@@ -810,17 +800,40 @@ getEndPointsOfDataDependentChain(SILValue value, SILFunction *fun,
       llvm::copy(user->getResults(), std::back_inserter(worklist));
     }
   }
-
   // At this point, visitedUsers have all the transitive, data-dependent uses.
-  // Compute the lifetime frontier of all the uses which are the instructions
-  // following the last uses. Every exit from the last uses will have a
-  // lifetime frontier.
+  users.append(visitedUsers.begin(), visitedUsers.end());
+}
+
+/// Collect the end points of the instructions that are data dependent on \c
+/// value. A instruction is data dependent on \c value if its result may
+/// transitively depends on \c value. Note that data dependencies through
+/// addresses are not tracked by this function.
+///
+/// \param value SILValue that is not an address.
+/// \param fun SILFunction that defines \c value.
+/// \param endUsers buffer for storing the found end points of the data
+/// dependence chain.
+static void
+getEndPointsOfDataDependentChain(SILValue value, SILFunction *fun,
+                                 SmallVectorImpl<SILInstruction *> &endUsers) {
+  assert(!value->getType().isAddress());
+
+  SmallVector<SILInstruction *, 16> transitiveUsers;
+  // Get transitive users of value, ignoring use-def chain going through
+  // branches. These transitive users define the end points of the constant
+  // evaluation. Igoring use-def chains through branches causes constant
+  // evaluation to miss some constant folding opportunities. This can be
+  // relaxed in the future, if necessary.
+  getTransitiveUsers(value, transitiveUsers);
+
+  // Compute the lifetime frontier of all the transitive uses which are the
+  // instructions following the last uses. Every exit from the last uses will
+  // have a lifetime frontier.
   SILInstruction *valueDefinition = value->getDefiningInstruction();
   SILInstruction *def =
       valueDefinition ? valueDefinition : &(value->getParentBlock()->front());
   ValueLifetimeAnalysis lifetimeAnalysis =
-      ValueLifetimeAnalysis(def, SmallVector<SILInstruction *, 16>(
-                                     visitedUsers.begin(), visitedUsers.end()));
+      ValueLifetimeAnalysis(def, transitiveUsers);
   ValueLifetimeAnalysis::Frontier frontier;
   bool hasCriticlEdges = lifetimeAnalysis.computeFrontier(
       frontier, ValueLifetimeAnalysis::DontModifyCFG);
@@ -936,7 +949,7 @@ static void replaceAllUsesAndFixLifetimes(SILValue foldedVal,
 static void substituteConstants(FoldState &foldState) {
   ConstExprStepEvaluator &evaluator = foldState.constantEvaluator;
   // Instructions that are possibly dead since their results are folded.
-  SmallVector<SILInstruction *, 4> possiblyDeadInsts;
+  SmallVector<SILInstruction *, 8> possiblyDeadInsts;
 
   for (SILValue constantSILValue : foldState.getConstantSILValues()) {
     SymbolicValue constantSymbolicVal =
@@ -980,13 +993,11 @@ static void substituteConstants(FoldState &foldState) {
     replaceAllUsesAndFixLifetimes(foldedSILVal, constantSILValue, fun);
     possiblyDeadInsts.push_back(definingInst);
   }
-  recursivelyDeleteTriviallyDeadInstructions(possiblyDeadInsts, /*force*/ false,
-                                             [&](SILInstruction *DeadI) {});
 }
 
 /// Check whether OSLogMessage and OSLogInterpolation instances and all their
 /// stored properties are constants. If not, it indicates errors that are due to
-/// incorrect implementation OSLogMessage either in the overlay or in the
+/// incorrect implementation of OSLogMessage either in the overlay or in the
 /// extensions created by users. Detect and emit diagnostics for such errors.
 /// The diagnostics here are for os log library authors.
 static bool checkOSLogMessageIsConstant(SingleValueInstruction *osLogMessage,
@@ -1048,6 +1059,37 @@ static bool checkOSLogMessageIsConstant(SingleValueInstruction *osLogMessage,
   return errorDetected;
 }
 
+/// Try to dead-code eliminate the OSLogMessage instance \c oslogMessage passed
+/// to the os log call and clean up its dependencies. If the instance cannot be
+/// eliminated, it implies that either the instance is not auto-generated or the
+/// implementation of the os log overlay is incorrect. Therefore emit
+/// diagnostics in such cases.
+static void tryEliminateOSLogMessage(SingleValueInstruction *oslogMessage) {
+  // Collect the set of root instructions that could be dead due to constant
+  // folding. These include the oslogMessage initialzer call and its transitive
+  // users.
+  SmallVector<SILInstruction *, 8> oslogMessageUsers;
+  getTransitiveUsers(oslogMessage, oslogMessageUsers);
+
+  InstructionDeleter deleter;
+  for (SILInstruction *user : oslogMessageUsers)
+    deleter.trackIfDead(user);
+  deleter.trackIfDead(oslogMessage);
+
+  bool isOSLogMessageDead = false;
+  deleter.cleanUpDeadInstructions([&](SILInstruction *deadInst) {
+    if (deadInst == oslogMessage)
+      isOSLogMessageDead = true;
+  });
+  // At this point, the OSLogMessage instance must be deleted if
+  // the overlay implementation (or its extensions by users) is correct.
+  if (!isOSLogMessageDead) {
+    SILFunction *fun = oslogMessage->getFunction();
+    diagnose(fun->getASTContext(), oslogMessage->getLoc().getSourceLoc(),
+             diag::oslog_message_alive_after_opts);
+  }
+}
+
 /// Constant evaluate instructions starting from 'start' and fold the uses
 /// of the value 'oslogMessage'. Stop when oslogMessageValue is released.
 static bool constantFold(SILInstruction *start,
@@ -1076,6 +1118,8 @@ static bool constantFold(SILInstruction *start,
     return false;
 
   substituteConstants(state);
+
+  tryEliminateOSLogMessage(oslogMessage);
   return true;
 }
 

lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp

@@ -2118,7 +2118,7 @@ bool AllocOptimize::promoteLoadCopy(LoadInst *li) {
     SILValue addr = li->getOperand();
     li->eraseFromParent();
     if (auto *addrI = addr->getDefiningInstruction())
-      recursivelyDeleteTriviallyDeadInstructions(addrI);
+      eliminateDeadInstruction(addrI);
     return true;
   }
 
@@ -2139,7 +2139,7 @@ bool AllocOptimize::promoteLoadCopy(LoadInst *li) {
   SILValue addr = li->getOperand();
   li->eraseFromParent();
   if (auto *addrI = addr->getDefiningInstruction())
-    recursivelyDeleteTriviallyDeadInstructions(addrI);
+    eliminateDeadInstruction(addrI);
   return true;
 }
 
@@ -2242,7 +2242,7 @@ bool AllocOptimize::promoteLoadBorrow(LoadBorrowInst *lbi) {
   SILValue addr = lbi->getOperand();
   lbi->eraseFromParent();
   if (auto *addrI = addr->getDefiningInstruction())
-    recursivelyDeleteTriviallyDeadInstructions(addrI);
+    eliminateDeadInstruction(addrI);
   return true;
 }
 

lib/SILOptimizer/Mandatory/SILGenCleanup.cpp

@@ -52,14 +52,12 @@ struct SILGenCanonicalize final : CanonicalizeInstruction {
       SILInstruction *deadOperInst = *deadOperands.begin();
       // Make sure at least the first instruction is removed from the set.
       deadOperands.erase(deadOperInst);
-      recursivelyDeleteTriviallyDeadInstructions(
-        deadOperInst, false,
-        [&](SILInstruction *deadInst) {
-          LLVM_DEBUG(llvm::dbgs() << "Trivially dead: " << *deadInst);
-          if (nextII == deadInst->getIterator())
-            ++nextII;
-          deadOperands.erase(deadInst);
-        });
+      eliminateDeadInstruction(deadOperInst, [&](SILInstruction *deadInst) {
+        LLVM_DEBUG(llvm::dbgs() << "Trivially dead: " << *deadInst);
+        if (nextII == deadInst->getIterator())
+          ++nextII;
+        deadOperands.erase(deadInst);
+      });
     }
     return nextII;
   }