Add false positive detection to sil lost variables

This patch adds false positive detection to sil-stats-lost-variables.
We will now only detect a debug_value as lost if there is a real
instruction which belongs to the same scope or a child scope of the
scope of the debug_value and if they are both inline at the same
location.

//a

Author: rastogishubham

lib/SILOptimizer/Utils/OptimizerStatsUtils.cpp

@@ -276,15 +276,22 @@ llvm::cl::opt<std::string> StatsOnlyFunctionsNamePattern(
 struct FunctionStat {
   int BlockCount = 0;
   int InstCount = 0;
+  /// True when the FunctionStat is created for a SIL Function after a SIL
+  /// Optimization pass has been run on it. When it is a post optimization SIL
+  /// Function, we do not want to store anything in the InlinedAts Map in
+  /// InstCountVisitor.
+  bool NewFunc = false;
   /// Instruction counts per SILInstruction kind.
   InstructionCounts InstCounts;
 
   using VarID = std::tuple<const SILDebugScope *, llvm::StringRef,
                            unsigned, unsigned>;
   llvm::StringSet<> VarNames;
   llvm::DenseSet<FunctionStat::VarID> DebugVariables;
+  llvm::DenseSet<const SILDebugScope *> VisitedScope;
+  llvm::DenseMap<VarID, const SILDebugScope *> InlinedAts;
 
-  FunctionStat(SILFunction *F);
+  FunctionStat(SILFunction *F, bool NewFunc = false);
   FunctionStat() {}
 
   // The DebugVariables set contains pointers to VarNames. Disallow copy.
@@ -384,15 +391,24 @@ struct ModuleStat {
 struct InstCountVisitor : SILInstructionVisitor<InstCountVisitor> {
   int BlockCount = 0;
   int InstCount = 0;
+  /// True when the InstCountVisitor is created for a SIL Function after a SIL
+  /// Optimization pass has been run on it. When it is a post optimization SIL
+  /// Function, we do not want to store anything in the InlinedAts Map in
+  /// InstCountVisitor.
+  const bool &NewFunc;
   InstructionCounts &InstCounts;
 
   llvm::StringSet<> &VarNames;
   llvm::DenseSet<FunctionStat::VarID> &DebugVariables;
+  llvm::DenseMap<FunctionStat::VarID, const SILDebugScope *> &InlinedAts;
 
-  InstCountVisitor(InstructionCounts &InstCounts,
-                   llvm::StringSet<> &VarNames,
-                   llvm::DenseSet<FunctionStat::VarID> &DebugVariables)
-  : InstCounts(InstCounts), VarNames(VarNames), DebugVariables(DebugVariables) {}
+  InstCountVisitor(
+      InstructionCounts &InstCounts, llvm::StringSet<> &VarNames,
+      llvm::DenseSet<FunctionStat::VarID> &DebugVariables,
+      llvm::DenseMap<FunctionStat::VarID, const SILDebugScope *> &InlinedAts,
+      bool &NewFunc)
+      : NewFunc(NewFunc), InstCounts(InstCounts), VarNames(VarNames),
+        DebugVariables(DebugVariables), InlinedAts(InlinedAts) {}
 
   int getBlockCount() const {
     return BlockCount;
@@ -431,6 +447,8 @@ struct InstCountVisitor : SILInstructionVisitor<InstCountVisitor> {
               varInfo->Scope ? varInfo->Scope : inst->getDebugScope(),
               UniqueName, line, col);
     DebugVariables.insert(key);
+    if (!NewFunc)
+      InlinedAts.try_emplace(key, varInfo->Scope->InlinedCallSite);
   }
 };
 
@@ -710,16 +728,85 @@ bool isFirstTimeData(int Old, int New) {
   return Old == 0 && New != Old;
 }
 
-int computeLostVariables(llvm::DenseSet<FunctionStat::VarID> &Old,
-                         llvm::DenseSet<FunctionStat::VarID> &New) {
-  unsigned count = 0;
-  for (auto &var : Old) {
-    if (New.contains(var))
+/// Return true if \p Scope is the same as \p DbgValScope or a child scope of
+/// \p DbgValScope, return false otherwise.
+bool isScopeChildOfOrEqualTo(const SILDebugScope *Scope,
+                             const SILDebugScope *DbgValScope) {
+  llvm::DenseSet<const SILDebugScope *> VisitedScope;
+  while (Scope != nullptr) {
+    if (VisitedScope.find(Scope) == VisitedScope.end()) {
+      VisitedScope.insert(Scope);
+      if (Scope == DbgValScope) {
+        return true;
+      }
+      if (auto *S = dyn_cast<const SILDebugScope *>(Scope->Parent))
+        Scope = S;
+    } else
+      return false;
+  }
+  return false;
+}
+
+/// Return true if \p InlinedAt is the same as \p DbgValInlinedAt or part of
+/// the InlinedAt chain, return false otherwise.
+bool isInlinedAtChildOfOrEqualTo(const SILDebugScope *InlinedAt,
+                                 const SILDebugScope *DbgValInlinedAt) {
+  if (DbgValInlinedAt == InlinedAt)
+    return true;
+  if (!DbgValInlinedAt)
+    return false;
+  if (!InlinedAt)
+    return false;
+  auto *IA = InlinedAt;
+  while (IA) {
+    if (IA == DbgValInlinedAt)
+      return true;
+    IA = IA->InlinedCallSite;
+  }
+  return false;
+}
+
+int computeLostVariables(SILFunction *F, FunctionStat &Old, FunctionStat &New) {
+  unsigned LostCount = 0;
+  unsigned PrevLostCount = 0;
+  auto &OldSet = Old.DebugVariables;
+  auto &NewSet = New.DebugVariables;
+  // Find an Instruction that shares the same scope as the dropped debug_value
+  // or has a scope that is the child of the scope of the debug_value, and has
+  // an inlinedAt equal to the inlinedAt of the debug_value or it's inlinedAt
+  // chain contains the inlinedAt of the debug_value, if such an Instruction is
+  // found, debug information is dropped.
+  for (auto &Var : OldSet) {
+    if (NewSet.contains(Var))
       continue;
-    // llvm::dbgs() << "Lost variable: " << std::get<1>(var) << "\n";
-    count++;
+    auto &DbgValScope = std::get<0>(Var);
+    for (auto &BB : *F) {
+      for (auto &I : BB) {
+        if (I.isDebugInstruction()) {
+          auto DbgLoc = I.getDebugLocation();
+          auto Scope = DbgLoc.getScope();
+          // If the Scope is a child of, or equal to the DbgValScope and is
+          // inlined at the Var's InlinedAt location, return true to signify
+          // that the Var has been dropped.
+          if (isScopeChildOfOrEqualTo(Scope, DbgValScope)) {
+            if (isInlinedAtChildOfOrEqualTo(Scope->InlinedCallSite,
+                                            Old.InlinedAts[Var])) {
+              // Found another instruction in the variable's scope, so there
+              // exists a break point at which the variable could be observed.
+              // Count it as dropped.
+              LostCount++;
+              break;
+            }
+          }
+        }
+      }
+      if (PrevLostCount != LostCount) {
+        PrevLostCount = LostCount;
+        break;
+      }
+    }
   }
-  return count;
+  return LostCount;
 }
 
 /// Dump statistics for a SILFunction. It is only used if a user asked to
@@ -782,8 +869,7 @@ void processFuncStatsChanges(SILFunction *F, FunctionStat &OldStat,
   // Compute deltas.
   double DeltaBlockCount = computeDelta(OldStat.BlockCount, NewStat.BlockCount);
   double DeltaInstCount = computeDelta(OldStat.InstCount, NewStat.InstCount);
-  int LostVariables = computeLostVariables(OldStat.DebugVariables,
-                                           NewStat.DebugVariables);
+  int LostVariables = computeLostVariables(F, OldStat, NewStat);
 
   NewLineInserter nl;
 
@@ -959,7 +1045,7 @@ void OptimizerStatsAnalysis::updateModuleStats(TransformationContext &Ctx) {
       InvalidatedFuncs.pop_back();
       auto &FuncStat = getFunctionStat(F);
       auto &OldFuncStat = FuncStat;
-      FunctionStat NewFuncStat(F);
+      FunctionStat NewFuncStat(F, true);
       processFuncStatsChanges(F, OldFuncStat, NewFuncStat, Ctx);
       NewModStat.subFunctionStat(OldFuncStat);
       NewModStat.addFunctionStat(NewFuncStat);
@@ -984,7 +1070,7 @@ void OptimizerStatsAnalysis::updateModuleStats(TransformationContext &Ctx) {
       AddedFuncs.pop_back();
       auto &FuncStat = getFunctionStat(F);
       FunctionStat OldFuncStat;
-      FunctionStat NewFuncStat(F);
+      FunctionStat NewFuncStat(F, true);
       processFuncStatsChanges(F, OldFuncStat, NewFuncStat, Ctx);
       NewModStat.addFunctionStat(NewFuncStat);
       FuncStat = std::move(NewFuncStat);
@@ -1003,8 +1089,8 @@ void OptimizerStatsAnalysis::updateModuleStats(TransformationContext &Ctx) {
   ModStat = NewModStat;
 }
 
-FunctionStat::FunctionStat(SILFunction *F) {
-  InstCountVisitor V(InstCounts, VarNames, DebugVariables);
+FunctionStat::FunctionStat(SILFunction *F, bool NewFunc) : NewFunc(NewFunc) {
+  InstCountVisitor V(InstCounts, VarNames, DebugVariables, InlinedAts, NewFunc);
   V.visitSILFunction(F);
   BlockCount = V.getBlockCount();
   InstCount = V.getInstCount();

test/DebugInfo/dropped-var.sil

@@ -0,0 +1,97 @@
+ // RUN: %target-sil-opt --diagnose-unreachable -o - %s -sil-stats-lost-variables 2>&1 | %FileCheck %s
+
+ // CHECK: function, lostvars, Pass List Pipeline, DiagnoseUnreachable, 1, 1, {{[0-9]+}}, $s4test3bar1yS2i_tF
+
+sil_stage raw
+
+import Builtin
+import Swift
+import SwiftShims
+
+sil_scope 1 { loc "file.swift":1:6 parent @$s4test3bar1yS2i_tF : $@convention(thin) (Int) -> Int }
+
+// main
+// Isolation: unspecified
+sil [ossa] @main : $@convention(c) (Int32, UnsafeMutablePointer<Optional<UnsafeMutablePointer<Int8>>>) -> Int32 {
+bb0(%0 : $Int32, %1 : $UnsafeMutablePointer<Optional<UnsafeMutablePointer<Int8>>>):
+  %2 = integer_literal $Builtin.Int32, 0          // user: %3;  auto_gen
+  %3 = struct $Int32 (%2 : $Builtin.Int32)        // user: %4;  auto_gen
+  return %3 : $Int32                              // id: %4;  auto_gen
+} // end sil function 'main'
+
+// bar(y:)
+// Isolation: unspecified
+sil hidden [ossa] @$s4test3bar1yS2i_tF : $@convention(thin) (Int) -> Int {
+// %0 "y"                                         // users: %5, %1
+bb0(%0 : $Int):
+  debug_value %0 : $Int, let, name "y", argno 1   // id: %1; line:1:10:in_prologue
+  %2 = integer_literal $Builtin.Int1, 0           // user: %3; line:2:11:minlined
+  cond_br %2, bb1, bb2                            // id: %3; line:2:11
+
+bb1:                                              // Preds: bb0
+  %4 = integer_literal $Builtin.Int64, 1          // user: %7; line:3:20:minlined
+  debug_value %4 : $Builtin.Int64, let, name "num"
+  %5 = struct_extract %0 : $Int, #Int._value      // user: %7; line:3:18:minlined
+  %6 = integer_literal $Builtin.Int1, -1          // user: %7; line:3:18:minlined
+  %7 = builtin "sadd_with_overflow_Int64"(%5 : $Builtin.Int64, %4 : $Builtin.Int64, %6 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1) // user: %8; line:3:18:minlined
+  (%8, %9) = destructure_tuple %7 : $(Builtin.Int64, Builtin.Int1) // users: %11, %10; line:3:18:minlined
+  cond_fail %9 : $Builtin.Int1, "arithmetic overflow" // id: %10; line:3:18:minlined
+  %11 = struct $Int (%8 : $Builtin.Int64)         // user: %12; line:3:18:minlined
+  br bb3(%11 : $Int)                              // id: %12; line:3:9:return
+bb2:                                              // Preds: bb0
+  %13 = integer_literal $Builtin.Int64, 1         // user: %14; line:5:12:minlined
+  %14 = struct $Int (%13 : $Builtin.Int64)        // user: %15; line:5:12:minlined
+  br bb3(%14 : $Int)                              // id: %15; line:5:5:return
+
+// %16                                            // user: %17
+bb3(%16 : $Int):                                  // Preds: bb2 bb1
+  return %16 : $Int                               // id: %17; line:6:1:cleanup
+
+// Do not count as dropped variable if there is no instruction that belongs to the same scope as lost debug_value
+bb4:
+  %17 = integer_literal $Builtin.Int64, 1, loc "file.swift":1:6, scope 1
+  debug_value %17 : $Builtin.Int64, let, name "num", loc "file.swift":1:6, scope 1
+  %18 = struct $Int (%17 : $Builtin.Int64)
+  br bb3(%18: $Int)
+
+} // end sil function '$s4test3bar1yS2i_tF'
+
+// Int.init(_builtinIntegerLiteral:)
+// Isolation: unspecified
+sil public_external [transparent] [serialized] [canonical] [ossa] @$sSi22_builtinIntegerLiteralSiBI_tcfC : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int {
+// %0                                             // user: %2
+bb0(%0 : $Builtin.IntLiteral, %1 : $@thin Int.Type):
+  %2 = builtin "s_to_s_checked_trunc_IntLiteral_Int64"(%0 : $Builtin.IntLiteral) : $(Builtin.Int64, Builtin.Int1) // user: %3;  no_loc
+  (%3, %4) = destructure_tuple %2 : $(Builtin.Int64, Builtin.Int1) // user: %5;  no_loc
+  %5 = struct $Int (%3 : $Builtin.Int64)          // user: %6;  no_loc
+  return %5 : $Int                                // id: %6;  no_loc
+} // end sil function '$sSi22_builtinIntegerLiteralSiBI_tcfC'
+
+// static Int.> infix(_:_:)
+// Isolation: unspecified
+sil public_external [transparent] [serialized] [canonical] [ossa] @$sSi1goiySbSi_SitFZ : $@convention(method) (Int, Int, @thin Int.Type) -> Bool {
+// %0                                             // user: %4
+// %1                                             // user: %3
+bb0(%0 : $Int, %1 : $Int, %2 : $@thin Int.Type):
+  %3 = struct_extract %1 : $Int, #Int._value      // user: %5;  no_loc
+  %4 = struct_extract %0 : $Int, #Int._value      // user: %5;  no_loc
+  %5 = builtin "cmp_slt_Int64"(%3 : $Builtin.Int64, %4 : $Builtin.Int64) : $Builtin.Int1 // user: %6;  no_loc
+  %6 = struct $Bool (%5 : $Builtin.Int1)          // user: %7;  no_loc
+  return %6 : $Bool                               // id: %7;  no_loc
+} // end sil function '$sSi1goiySbSi_SitFZ'
+
+// static Int.+ infix(_:_:)
+// Isolation: unspecified
+sil public_external [transparent] [serialized] [canonical] [ossa] @$sSi1poiyS2i_SitFZ : $@convention(method) (Int, Int, @thin Int.Type) -> Int {
+// %0                                             // user: %3
+// %1                                             // user: %4
+bb0(%0 : $Int, %1 : $Int, %2 : $@thin Int.Type):
+  %3 = struct_extract %0 : $Int, #Int._value      // user: %6;  no_loc
+  %4 = struct_extract %1 : $Int, #Int._value      // user: %6;  no_loc
+  %5 = integer_literal $Builtin.Int1, -1          // user: %6;  no_loc
+  %6 = builtin "sadd_with_overflow_Int64"(%3 : $Builtin.Int64, %4 : $Builtin.Int64, %5 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1) // user: %7;  no_loc
+  (%7, %8) = destructure_tuple %6 : $(Builtin.Int64, Builtin.Int1) // users: %10, %9;  no_loc
+  cond_fail %8 : $Builtin.Int1, "arithmetic overflow" // id: %9;  no_loc
+  %10 = struct $Int (%7 : $Builtin.Int64)         // user: %11;  no_loc
+  return %10 : $Int                               // id: %11;  no_loc
+} // end sil function '$sSi1poiyS2i_SitFZ'