[nfc][opt] Add more stack traces and make them more specific.

Author: zoecarver

lib/SILOptimizer/Mandatory/PerformanceDiagnostics.cpp

@@ -15,7 +15,6 @@
 #include "swift/AST/SemanticAttrs.h"
 #include "swift/SIL/BasicBlockDatastructures.h"
 #include "swift/SIL/InstructionUtils.h"
-#include "swift/SIL/PrettyStackTrace.h"
 #include "swift/SIL/ApplySite.h"
 #include "swift/SILOptimizer/Analysis/ArraySemantic.h"
 #include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
@@ -27,6 +26,41 @@ using namespace swift;
 
 namespace {
 
+class PrettyStackTracePerformanceDiagnostics
+  : public llvm::PrettyStackTraceEntry {
+  const SILNode *node;
+  const char *action;
+
+public:
+  PrettyStackTracePerformanceDiagnostics(const char *action, SILNodePointer node)
+    : node(node), action(action) {}
+
+  virtual void print(llvm::raw_ostream &OS) const override {
+    OS << "While " << action << " -- visiting node ";
+    node->print(OS);
+    
+    if (auto inst = dyn_cast<SILInstruction>(node)) {
+      OS << "While visiting instruction in function ";
+      inst->getFunction()->print(OS);
+    }
+  }
+};
+
+class PrettyStackTraceSILGlobal
+  : public llvm::PrettyStackTraceEntry {
+  const SILGlobalVariable *node;
+  const char *action;
+
+public:
+    PrettyStackTraceSILGlobal(const char *action, SILGlobalVariable *node)
+    : node(node), action(action) {}
+
+  virtual void print(llvm::raw_ostream &OS) const override {
+    OS << "While " << action << " -- visiting node ";
+    node->print(OS);
+  }
+};
+
 /// Issues performance diagnostics for functions which are annotated with
 /// performance annotations, like @_noLocks, @_noAllocation.
 ///
@@ -120,9 +154,6 @@ static bool isEffectFreeArraySemanticCall(SILInstruction *inst) {
 bool PerformanceDiagnostics::visitFunction(SILFunction *function,
                                               PerformanceConstraints perfConstr,
                                               LocWithParent *parentLoc) {
-  PrettyStackTraceSILFunction stackTrace(
-      "Running performance diangostics on (visiting) ", function);
-
   if (!function->isDefinition())
     return false;
 
@@ -160,8 +191,9 @@ bool PerformanceDiagnostics::visitFunction(SILFunction *function,
         if (visitCallee(&inst, bca->getCalleeList(as), perfConstr, parentLoc))
           return true;
       } else if (auto *bi = dyn_cast<BuiltinInst>(&inst)) {
-        PrettyStackTraceSILNode biStackTrace(
-            "Validating built in (once, once with context)", bi);
+        PrettyStackTracePerformanceDiagnostics stackTrace(
+            "visitFunction::BuiltinInst (once, once with context)", &inst);
+
         switch (bi->getBuiltinInfo().ID) {
           case BuiltinValueKind::Once:
           case BuiltinValueKind::OnceWithContext:
@@ -210,8 +242,6 @@ bool PerformanceDiagnostics::checkClosureValue(SILValue closure,
                                             SILInstruction *callInst,
                                             PerformanceConstraints perfConstr,
                                             LocWithParent *parentLoc) {
-  PrettyStackTraceSILNode closureStackTrace("Validating closure", closure);
-
   // Walk through the definition of the closure until we find the "underlying"
   // function_ref instruction.
   while (!isa<FunctionRefInst>(closure)) {
@@ -234,6 +264,9 @@ bool PerformanceDiagnostics::checkClosureValue(SILValue closure,
       // the call site.
       return false;
     } else {
+      PrettyStackTracePerformanceDiagnostics stackTrace(
+          "validating closure (function ref, callee) - unkown callee", callInst);
+
       diagnose(LocWithParent(callInst->getLoc().getSourceLoc(), parentLoc), diag::performance_unknown_callees);
       return true;
     }
@@ -251,14 +284,14 @@ bool PerformanceDiagnostics::visitCallee(SILInstruction *callInst,
                                          CalleeList callees,
                                          PerformanceConstraints perfConstr,
                                          LocWithParent *parentLoc) {
-  PrettyStackTraceSILNode callStackTrace("Validating callee", callInst);
-
   LocWithParent asLoc(callInst->getLoc().getSourceLoc(), parentLoc);
   LocWithParent *loc = &asLoc;
   if (parentLoc && asLoc.loc == callInst->getFunction()->getLocation().getSourceLoc())
     loc = parentLoc;
 
   if (callees.isIncomplete()) {
+    PrettyStackTracePerformanceDiagnostics stackTrace("incomplete", callInst);
+
     diagnose(*loc, diag::performance_unknown_callees);
     return true;
   }
@@ -273,6 +306,8 @@ bool PerformanceDiagnostics::visitCallee(SILInstruction *callInst,
       return false;
 
     if (!callee->isDefinition()) {
+      PrettyStackTracePerformanceDiagnostics stackTrace("incomplete", callInst);
+
       diagnose(*loc, diag::performance_callee_unavailable);
       return true;
     }
@@ -291,8 +326,6 @@ bool PerformanceDiagnostics::visitCallee(SILInstruction *callInst,
 }
 
 static bool metatypeUsesAreNotRelevant(MetatypeInst *mt) {
-  PrettyStackTraceSILNode mtStackTrace("Validating metatype", mt);
-
   for (Operand *use : mt->getUses()) {
     if (auto  *bi = dyn_cast<BuiltinInst>(use->getUser())) {
       switch (bi->getBuiltinInfo().ID) {
@@ -315,15 +348,15 @@ static bool metatypeUsesAreNotRelevant(MetatypeInst *mt) {
 bool PerformanceDiagnostics::visitInst(SILInstruction *inst,
                                           PerformanceConstraints perfConstr,
                                           LocWithParent *parentLoc) {
-  PrettyStackTraceSILNode stackTrace("validating sil instruction (visiting)", inst);
-
   SILType impactType;
   RuntimeEffect impact = getRuntimeEffect(inst, impactType);
   LocWithParent loc(inst->getLoc().getSourceLoc(), parentLoc);
 
   if (impact & RuntimeEffect::Casting) {
     // TODO: be more specific on casting.
     // E.g. distinguish locking and allocating dynamic casts, etc.
+    PrettyStackTracePerformanceDiagnostics stackTrace("bad cast", inst);
+
     diagnose(loc, diag::performance_dynamic_casting);
     return true;
   }
@@ -333,21 +366,30 @@ bool PerformanceDiagnostics::visitInst(SILInstruction *inst,
 
     // Try to give a good error message by looking which type of code it is.
     switch (inst->getKind()) {
-    case SILInstructionKind::KeyPathInst:
+    case SILInstructionKind::KeyPathInst: {
+      PrettyStackTracePerformanceDiagnostics stackTrace("key path", inst);
       diagnose(loc, diag::performance_metadata, "using KeyPath");
       break;
+    }
     case SILInstructionKind::AllocGlobalInst:
-    case SILInstructionKind::GlobalValueInst:
+    case SILInstructionKind::GlobalValueInst: {
+      PrettyStackTracePerformanceDiagnostics stackTrace(
+            "AllocGlobalInst | GlobalValueInst", inst);
       diagnose(loc, diag::performance_metadata, "global or static variables");
       break;
+    }
     case SILInstructionKind::PartialApplyInst: {
+      PrettyStackTracePerformanceDiagnostics stackTrace(
+          "PartialApplyInst", inst);
       diagnose(loc, diag::performance_metadata,
                "generic closures or local functions");
       break;
     }
     case SILInstructionKind::ApplyInst:
     case SILInstructionKind::TryApplyInst:
     case SILInstructionKind::BeginApplyInst: {
+      PrettyStackTracePerformanceDiagnostics stackTrace(
+          "ApplyInst | TryApplyInst | BeginApplyInst", inst);
       diagnose(loc, diag::performance_metadata, "generic function calls");
       break;
     }
@@ -359,16 +401,23 @@ bool PerformanceDiagnostics::visitInst(SILInstruction *inst,
       // We didn't recognize the instruction, so try to give an error message
       // based on the involved type.
       if (impactType) {
+        PrettyStackTracePerformanceDiagnostics stackTrace(
+            "impactType (unrecognized instruction)", inst);
         diagnose(loc, diag::performance_metadata_type, impactType.getASTType());
         break;
       }
       // The default error message.
+      PrettyStackTracePerformanceDiagnostics stackTrace(
+          "default error (fallthrough, unkown inst)", inst);
       diagnose(loc, diag::performance_metadata, "this code pattern");
       break;
     }
     return true;
   }
   if (impact & RuntimeEffect::Allocating) {
+    PrettyStackTracePerformanceDiagnostics stackTrace(
+        "found allocation effect", inst);
+
     switch (inst->getKind()) {
     case SILInstructionKind::BeginApplyInst:
       // Not all begin_applys necessarily allocate. But it's difficult to
@@ -389,6 +438,9 @@ bool PerformanceDiagnostics::visitInst(SILInstruction *inst,
     return true;
   }
   if (impact & RuntimeEffect::Deallocating) {
+    PrettyStackTracePerformanceDiagnostics stackTrace(
+        "found deallocation effect", inst);
+
     if (impactType) {
       switch (inst->getKind()) {
       case SILInstructionKind::StoreInst:
@@ -409,6 +461,9 @@ bool PerformanceDiagnostics::visitInst(SILInstruction *inst,
     return true;
   }
   if (impact & RuntimeEffect::ObjectiveC) {
+    PrettyStackTracePerformanceDiagnostics stackTrace(
+        "found objc effect", inst);
+
     diagnose(loc, diag::performance_objectivec);
     return true;
   }
@@ -417,6 +472,9 @@ bool PerformanceDiagnostics::visitInst(SILInstruction *inst,
     return false;
 
   // Handle locking-only effects.
+  
+  PrettyStackTracePerformanceDiagnostics stackTrace(
+      "found locking or ref counting effect", inst);
 
   if (impact & RuntimeEffect::Locking) {
     if (inst->getFunction()->isGlobalInit()) {
@@ -461,7 +519,7 @@ void PerformanceDiagnostics::checkNonAnnotatedFunction(SILFunction *function) {
   }
 }
 
-void PerformanceDiagnostics::diagnose(LocWithParent loc, Diagnostic &&D) {
+void PerformanceDiagnostics::diagnose(LocWithParent loc, Diagnostic &&D) {  
   // Start with a valid location in the call tree.
   LocWithParent *validLoc = &loc;
   while (!validLoc->loc.isValid() && validLoc->parent) {
@@ -498,6 +556,9 @@ class PerformanceDiagnosticsPass : public SILModuleTransform {
     // Check that @_section, @_silgen_name is only on constant globals
     for (SILGlobalVariable &g : module->getSILGlobals()) {
       if (!g.getStaticInitializerValue() && g.mustBeInitializedStatically()) {
+        PrettyStackTraceSILGlobal stackTrace(
+            "global inst", &g);
+
         auto *decl = g.getDecl();
         if (g.getSectionAttr()) {
           module->getASTContext().Diags.diagnose(