Merge pull request swiftlang#33023 from gottesmm/pr-c7a2ff52b8850a874dcf6dc84616594e4cbcfaf4

[opt-remark] Teach opt-remark how to emit back notes to retain/released variables in simple cases.

Author: gottesmm

include/swift/AST/DiagnosticsSIL.def

@@ -536,6 +536,7 @@ ERROR(non_borrowed_indirect_addressof,none,
 
 REMARK(opt_remark_passed, none, "%0", (StringRef))
 REMARK(opt_remark_missed, none, "%0", (StringRef))
+NOTE(opt_remark_note, none, "%0", (StringRef))
 
 // Float-point to integer conversions
 ERROR(float_to_int_overflow, none,

include/swift/SIL/OptimizationRemark.h

@@ -21,6 +21,7 @@
 
 #include "swift/Basic/SourceLoc.h"
 #include "swift/Demangling/Demangler.h"
+#include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILBasicBlock.h"
 #include "swift/SIL/SILInstruction.h"
 #include "swift/SIL/SILModule.h"
@@ -32,27 +33,105 @@ class SILFunction;
 
 namespace OptRemark {
 
+struct ArgumentKeyKind {
+  enum InnerTy {
+    // Just assume this is a normal msg that we are emitting.
+    Default,
+
+    // Assume this is a note that should be emitted as a separate
+    // diagnostic when emitting diagnostics. Do nothing special
+    // along the backend path.
+    Note,
+
+    // Assume that this is a note that should be emitted as a separate
+    // diagnostic but that doesn't have its own source loc: we should reuse the
+    // one for the original remark.
+    //
+    // This is intended to be used in situations where one needs to emit a
+    // "note" warning due to us not being able to infer a part of our
+    // opt-remark.
+    ParentLocNote,
+  };
+
+  InnerTy innerValue;
+
+  ArgumentKeyKind(InnerTy value) : innerValue(value) {}
+  ArgumentKeyKind(const ArgumentKeyKind &kind) : innerValue(kind.innerValue) {}
+
+  operator InnerTy() const { return innerValue; }
+
+  /// Return true if this argument is meant to be a separate diagnostic when we
+  /// emit diagnostics but when we emit to the remark streamer (due to not
+  /// having support for this), we just emit the remark inline.
+  ///
+  /// TODO: Unfortunate that this needs to be done.
+  bool isSeparateDiagnostic() const {
+    switch (innerValue) {
+    case InnerTy::Default:
+      return false;
+    case InnerTy::Note:
+    case InnerTy::ParentLocNote:
+      return true;
+    }
+
+    llvm_unreachable("Covered switch isn't covered?!");
+  }
+};
+
+struct ArgumentKey {
+  ArgumentKeyKind kind;
+  std::string data;
+
+  ArgumentKey(ArgumentKeyKind kind, StringRef data) : kind(kind), data(data) {}
+  ArgumentKey(ArgumentKeyKind::InnerTy kind, StringRef data)
+      : kind(kind), data(data) {}
+  ArgumentKey(ArgumentKey kind, StringRef data) : kind(kind.kind), data(data) {}
+};
+
 /// Used in the streaming interface as the general argument type.  It
 /// internally converts everything into a key-value pair.
 struct Argument {
-  std::string key;
+  ArgumentKey key;
   std::string val;
   /// If set, the debug location corresponding to the value.
   SourceLoc loc;
 
-  explicit Argument(StringRef Str = "") : key("String"), val(Str) {}
-  Argument(StringRef key, StringRef val) : key(key), val(val) {}
+  explicit Argument(StringRef str = "")
+      : Argument({ArgumentKeyKind::Default, "String"}, str) {}
 
+  Argument(StringRef key, StringRef val)
+      : Argument({ArgumentKeyKind::Default, key}, val) {}
+  Argument(ArgumentKey key, StringRef val) : key(key), val(val) {}
   Argument(StringRef key, int n);
   Argument(StringRef key, long n);
   Argument(StringRef key, long long n);
   Argument(StringRef key, unsigned n);
   Argument(StringRef key, unsigned long n);
   Argument(StringRef key, unsigned long long n);
 
-  Argument(StringRef key, SILFunction *f);
+  Argument(StringRef key, SILFunction *f)
+      : Argument(ArgumentKey(ArgumentKeyKind::Default, key), f) {}
+  Argument(ArgumentKey key, SILFunction *f);
   Argument(StringRef key, SILType ty);
   Argument(StringRef key, CanType ty);
+
+  Argument(StringRef key, StringRef msg, const ValueDecl *decl)
+      : Argument(ArgumentKey(ArgumentKeyKind::Default, key), msg, decl) {}
+  Argument(ArgumentKey key, StringRef msg, const ValueDecl *decl)
+      : key(key), val(msg), loc(decl->getLoc()) {}
+
+  /// Given a value, call \p funcPassedInferredArgs for each associated
+  /// ValueDecl that is associated with \p value. All created Arguments are
+  /// passed the same StringRef. To stop iteration, return false in \p
+  /// funcPassedInferedArgs.
+  ///
+  /// NOTE: the function may be called multiple times if the optimizer joined
+  /// two live ranges and thus when iterating over value's users we see multiple
+  /// debug_value operations.
+  static bool
+  inferArgumentsForValue(ArgumentKeyKind keyKind, StringRef message,
+                         SILValue value,
+                         function_ref<bool(Argument)> funcPassedInferedArgs);
 };
 
 /// Shorthand to insert named-value pairs.

include/swift/SIL/SILRemarkStreamer.h

@@ -111,7 +111,7 @@ llvm::remarks::Remark SILRemarkStreamer::toLLVMRemark(
 
   for (const OptRemark::Argument &arg : optRemark.getArgs()) {
     llvmRemark.Args.emplace_back();
-    llvmRemark.Args.back().Key = arg.key;
+    llvmRemark.Args.back().Key = arg.key.data;
     llvmRemark.Args.back().Val = arg.val;
     llvmRemark.Args.back().Loc =
         toRemarkLocation(arg.loc, getASTContext().SourceMgr);

lib/SIL/Utils/OptimizationRemark.cpp

@@ -20,6 +20,9 @@
 #include "swift/AST/DiagnosticEngine.h"
 #include "swift/AST/DiagnosticsSIL.h"
 #include "swift/Demangling/Demangler.h"
+#include "swift/SIL/DebugUtils.h"
+#include "swift/SIL/SILArgument.h"
+#include "swift/SIL/SILInstruction.h"
 #include "swift/SIL/SILRemarkStreamer.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/CommandLine.h"
@@ -28,23 +31,25 @@
 using namespace swift;
 using namespace OptRemark;
 
-Argument::Argument(StringRef key, int n) : key(key), val(llvm::itostr(n)) {}
+Argument::Argument(StringRef key, int n)
+    : key(ArgumentKeyKind::Default, key), val(llvm::itostr(n)) {}
 
-Argument::Argument(StringRef key, long n) : key(key), val(llvm::itostr(n)) {}
+Argument::Argument(StringRef key, long n)
+    : key(ArgumentKeyKind::Default, key), val(llvm::itostr(n)) {}
 
 Argument::Argument(StringRef key, long long n)
-    : key(key), val(llvm::itostr(n)) {}
+    : key(ArgumentKeyKind::Default, key), val(llvm::itostr(n)) {}
 
 Argument::Argument(StringRef key, unsigned n)
-    : key(key), val(llvm::utostr(n)) {}
+    : key(ArgumentKeyKind::Default, key), val(llvm::utostr(n)) {}
 
 Argument::Argument(StringRef key, unsigned long n)
-    : key(key), val(llvm::utostr(n)) {}
+    : key(ArgumentKeyKind::Default, key), val(llvm::utostr(n)) {}
 
 Argument::Argument(StringRef key, unsigned long long n)
-    : key(key), val(llvm::utostr(n)) {}
+    : key(ArgumentKeyKind::Default, key), val(llvm::utostr(n)) {}
 
-Argument::Argument(StringRef key, SILFunction *f) : key(key) {
+Argument::Argument(ArgumentKey key, SILFunction *f) : key(key) {
   auto options = Demangle::DemangleOptions::SimplifiedUIDemangleOptions();
   // Enable module names so that we have a way of filtering out
   // stdlib-related remarks.
@@ -58,22 +63,53 @@ Argument::Argument(StringRef key, SILFunction *f) : key(key) {
     loc = f->getLocation().getSourceLoc();
 }
 
-Argument::Argument(StringRef key, SILType ty) : key(key) {
+Argument::Argument(StringRef key, SILType ty)
+    : key(ArgumentKeyKind::Default, key) {
   llvm::raw_string_ostream stream(val);
   ty.print(stream);
 }
 
-Argument::Argument(StringRef key, CanType ty) : key(key) {
+Argument::Argument(StringRef key, CanType ty)
+    : key(ArgumentKeyKind::Default, key) {
   llvm::raw_string_ostream stream(val);
   ty.print(stream);
 }
 
+bool Argument::inferArgumentsForValue(
+    ArgumentKeyKind keyKind, StringRef msg, SILValue value,
+    function_ref<bool(Argument)> funcPassedInferedArgs) {
+  // If we have an argument, just use that.
+  if (auto *arg = dyn_cast<SILArgument>(value))
+    if (auto *decl = arg->getDecl())
+      return funcPassedInferedArgs(
+          Argument({keyKind, "InferredValue"}, msg, decl));
+
+  // TODO: Look for loads from globals and addresses.
+
+  // Otherwise, look for debug_values.
+  for (auto *use : getDebugUses(value))
+    if (auto *dvi = dyn_cast<DebugValueInst>(use->getUser()))
+      if (auto *decl = dvi->getDecl())
+        if (!funcPassedInferedArgs(
+                Argument({keyKind, "InferredValue"}, msg, decl)))
+          return false;
+
+  return true;
+}
+
 template <typename DerivedT>
 std::string Remark<DerivedT>::getMsg() const {
   std::string str;
   llvm::raw_string_ostream stream(str);
-  for (const Argument &arg : args)
+  // Go through our args and if we are not emitting for diagnostics *OR* we are
+  // emitting for diagnostics and this argument is not intended to be emitted as
+  // a diagnostic separate from our main remark, emit the arg value here.
+  for (const Argument &arg : args) {
+    if (arg.key.kind.isSeparateDiagnostic())
+      continue;
     stream << arg.val;
+  }
+
   return stream.str();
 }
 
@@ -194,9 +230,30 @@ static void emitRemark(SILModule &module, const Remark<RemarkT> &remark,
     return;
   if (auto *remarkStreamer = module.getSILRemarkStreamer())
     remarkStreamer->emit(remark);
-  if (diagEnabled)
-    module.getASTContext().Diags.diagnose(remark.getLocation(), id,
-                                          remark.getMsg());
+
+  // If diagnostics are enabled, first emit the main diagnostic and then loop
+  // through our arguments and allow the arguments to add additional diagnostics
+  // if they want.
+  if (!diagEnabled)
+    return;
+
+  auto &de = module.getASTContext().Diags;
+  de.diagnoseWithNotes(
+      de.diagnose(remark.getLocation(), id, remark.getMsg()), [&]() {
+        for (auto &arg : remark.getArgs()) {
+          switch (arg.key.kind) {
+          case ArgumentKeyKind::Default:
+            continue;
+          case ArgumentKeyKind::Note:
+            de.diagnose(arg.loc, diag::opt_remark_note, arg.val);
+            continue;
+          case ArgumentKeyKind::ParentLocNote:
+            de.diagnose(remark.getLocation(), diag::opt_remark_note, arg.val);
+            continue;
+          }
+          llvm_unreachable("Unhandled case?!");
+        }
+      });
 }
 
 void Emitter::emit(const RemarkPassed &remark) {