Merge pull request #40278 from atrick/fix-debuginfo

Author: swift-ci

include/swift/SILOptimizer/Utils/CanonicalizeInstruction.h

@@ -42,11 +42,16 @@ struct CanonicalizeInstruction {
   const char *debugType = defaultDebugType;
   DeadEndBlocks &deadEndBlocks;
   InstModCallbacks callbacks;
+  bool preserveDebugInfo;
 
   CanonicalizeInstruction(const char *passDebugType,
                           DeadEndBlocks &deadEndBlocks)
       : deadEndBlocks(deadEndBlocks),
         callbacks() {
+
+    preserveDebugInfo = getFunction()->getEffectiveOptimizationMode()
+      <= OptimizationMode::NoOptimization;
+
 #ifndef NDEBUG
     if (llvm::DebugFlag && !llvm::isCurrentDebugType(debugType))
       debugType = passDebugType;
@@ -68,6 +73,9 @@ struct CanonicalizeInstruction {
 
   const SILFunction *getFunction() const { return deadEndBlocks.getFunction(); }
 
+  // TODO: callbacks should come from the current InstructionDeleter.
+  InstModCallbacks &getCallbacks() { return callbacks; }
+
   /// Rewrite this instruction, based on its operands and uses, into a more
   /// canonical representation.
   ///

include/swift/SILOptimizer/Utils/DebugOptUtils.h

@@ -19,6 +19,7 @@
 #define SWIFT_SILOPTIMIZER_DEBUGOPTUTILS_H
 
 #include "swift/SIL/DebugUtils.h"
+#include "swift/SIL/Projection.h"
 #include "swift/SIL/SILValue.h"
 #include "swift/SILOptimizer/Utils/InstOptUtils.h"
 
@@ -47,6 +48,13 @@ inline void deleteAllDebugUses(SILInstruction *inst,
 /// new `debug_value` instruction before \p I is deleted.
 void salvageDebugInfo(SILInstruction *I);
 
+/// Create debug_value fragment for a new partial value.
+///
+/// Precondition: \p oldValue is a struct or class aggregate. \p proj projects a
+/// field from the aggregate into \p newValue corresponding to struct_extract.
+void createDebugFragments(SILValue oldValue, Projection proj,
+                          SILValue newValue);
+
 /// Erases the instruction \p I from it's parent block and deletes it, including
 /// all debug instructions which use \p I.
 /// Precondition: The instruction may only have debug instructions as uses.

include/swift/SILOptimizer/Utils/OwnershipOptUtils.h

@@ -345,23 +345,17 @@ struct LoadOperation {
 
   explicit operator bool() const { return !value.isNull(); }
 
-  SingleValueInstruction *operator*() const {
+  SingleValueInstruction *getLoadInst() const {
     if (value.is<LoadInst *>())
       return value.get<LoadInst *>();
     return value.get<LoadBorrowInst *>();
   }
 
-  const SingleValueInstruction *operator->() const {
-    if (value.is<LoadInst *>())
-      return value.get<LoadInst *>();
-    return value.get<LoadBorrowInst *>();
-  }
+  SingleValueInstruction *operator*() const { return getLoadInst(); }
 
-  SingleValueInstruction *operator->() {
-    if (value.is<LoadInst *>())
-      return value.get<LoadInst *>();
-    return value.get<LoadBorrowInst *>();
-  }
+  const SingleValueInstruction *operator->() const { return getLoadInst(); }
+
+  SingleValueInstruction *operator->() { return getLoadInst(); }
 
   SILValue getOperand() const {
     if (value.is<LoadInst *>())

lib/SILOptimizer/Transforms/DeadCodeElimination.cpp

@@ -581,9 +581,11 @@ bool DCE::removeDead() {
         // dead and will be deleted.
         // We do not have to end lifetime of a @guaranteed non phi arg.
         if (arg->getOwnershipKind() == OwnershipKind::Owned) {
+          auto loc = RegularLocation::getAutoGeneratedLocation();
+          // insertPt is non-null because Undef is non-owned.
           auto insertPt = getInsertAfterPoint(arg).getValue();
           SILBuilderWithScope builder(insertPt);
-          auto *destroy = builder.createDestroyValue(insertPt->getLoc(), arg);
+          auto *destroy = builder.createDestroyValue(loc, arg);
           LiveInstructions.insert(destroy);
         }
         i++;

lib/SILOptimizer/Utils/CanonicalizeInstruction.cpp

@@ -36,6 +36,10 @@ using namespace swift;
 // Tracing within the implementation can also be activiated by the pass.
 #define DEBUG_TYPE pass.debugType
 
+llvm::cl::opt<bool> EnableLoadSplittingDebugInfo(
+    "sil-load-splitting-debug-info", llvm::cl::init(false),
+    llvm::cl::desc("Create debug fragments at -O for partial loads"));
+
 // Vtable anchor.
 CanonicalizeInstruction::~CanonicalizeInstruction() {}
 
@@ -243,6 +247,9 @@ splitAggregateLoad(LoadOperation loadInst, CanonicalizeInstruction &pass) {
   //   }
   // }
   //
+  // Also, avoid degrading debug info unless it is necessary for exclusivity
+  // diagnostics.
+  //
   // TODO: This logic subtly anticipates SILGen behavior. In the future, change
   // SILGen to avoid emitting the full load and never delete loads in Raw SIL.
   if (projections.empty() && loadInst->getModule().getStage() == SILStage::Raw)
@@ -294,14 +301,27 @@ splitAggregateLoad(LoadOperation loadInst, CanonicalizeInstruction &pass) {
     }
     pass.notifyNewInstruction(**lastNewLoad);
 
+    // FIXME: This drops debug info at -Onone load-splitting is required at
+    // -Onone for exclusivity diagnostics. Fix this by
+    // 
+    // 1. At -Onone, preserve the original load when pass.preserveDebugInfo is
+    // true, but moving it out of its current access scope and into an "unknown"
+    // access scope, which won't be enforced as an exclusivity violation.
+    //
+    // 2. At -O, create "debug fragments" recover as much debug info as possible
+    // by creating debug_value fragments for each new partial load. Currently
+    // disabled because of LLVM back-end crashes.
+    if (!pass.preserveDebugInfo && EnableLoadSplittingDebugInfo) {
+      createDebugFragments(*loadInst, proj, lastNewLoad->getLoadInst());
+    }
     if (loadOwnership) {
       if (*loadOwnership == LoadOwnershipQualifier::Copy) {
         // Destroy the loaded value wherever the aggregate load was destroyed.
         assert(loadInst.getOwnershipQualifier() ==
                LoadOwnershipQualifier::Copy);
         for (SILInstruction *destroy : lifetimeEndingInsts) {
           auto *newInst = SILBuilderWithScope(destroy).createDestroyValue(
-              destroy->getLoc(), **lastNewLoad);
+              destroy->getLoc(), lastNewLoad->getLoadInst());
           pass.notifyNewInstruction(newInst);
         }
       }
@@ -324,6 +344,14 @@ splitAggregateLoad(LoadOperation loadInst, CanonicalizeInstruction &pass) {
   for (auto *destroy : lifetimeEndingInsts)
     nextII = killInstruction(destroy, nextII, pass);
 
+  // FIXME: remove this temporary hack to advance the iterator beyond
+  // debug_value. A soon-to-be merged commit migrates CanonicalizeInstruction to
+  // use InstructionDeleter.
+  while (nextII != loadInst->getParent()->end()
+         && nextII->isDebugInstruction()) {
+    ++nextII;
+  }
+  deleteAllDebugUses(*loadInst, pass.getCallbacks());
   return killInstAndIncidentalUses(*loadInst, nextII, pass);
 }
 
@@ -414,6 +442,14 @@ broadenSingleElementStores(StoreInst *storeInst,
 /// copy. The only way to guarantee the lifetime of a variable is to use a
 /// borrow scope--copy/destroy is insufficient by itself.
 ///
+/// FIXME: This removes debug_value instructions aggressively as part of
+/// SILGenCleanup. Instead, debug_values should be canonicalized before copy
+/// elimination so that we never see the pattern:
+///   %b = begin_borrow
+///   %c = copy %b
+///   end_borrow %b
+///   debug_value %c
+///
 /// FIXME: Technically this should be guarded by a compiler flag like
 /// -enable-copy-propagation until SILGen protects scoped variables by
 /// borrow scopes.
@@ -423,20 +459,21 @@ eliminateSimpleCopies(CopyValueInst *cvi, CanonicalizeInstruction &pass) {
 
   // Eliminate copies that only have destroy_value uses.
   SmallVector<DestroyValueInst *, 8> destroys;
-  for (auto *use : getNonDebugUses(cvi)) {
+  for (Operand *use : cvi->getUses()) {
     if (auto *dvi = dyn_cast<DestroyValueInst>(use->getUser())) {
       destroys.push_back(dvi);
       continue;
     }
+    if (!pass.preserveDebugInfo && isa<DebugValueInst>(use->getUser())) {
+      continue;
+    }
     return next;
   }
 
   while (!destroys.empty()) {
     next = killInstruction(destroys.pop_back_val(), next, pass);
   }
-
-  next = killInstAndIncidentalUses(cvi, next, pass);
-  return next;
+  return killInstAndIncidentalUses(cvi, next, pass);
 }
 
 /// Unlike dead copy elimination, dead borrows can be safely removed because the
@@ -524,7 +561,6 @@ CanonicalizeInstruction::canonicalize(SILInstruction *inst) {
   if (auto li = LoadOperation(inst)) {
     return splitAggregateLoad(li, *this);
   }
-
   if (auto *storeInst = dyn_cast<StoreInst>(inst)) {
     return broadenSingleElementStores(storeInst, *this);
   }
@@ -538,8 +574,11 @@ CanonicalizeInstruction::canonicalize(SILInstruction *inst) {
   // If we have ownership and are not in raw SIL, eliminate unneeded forwarding
   // insts. We don't do this in raw SIL as not to disturb the codegen read by
   // diagnostics.
+  //
+  // TODO: fix tryEliminateUnneededForwardingInst to handle debug uses.
   auto *fn = inst->getFunction();
-  if (fn->hasOwnership() && fn->getModule().getStage() != SILStage::Raw) {
+  if (!preserveDebugInfo && fn->hasOwnership()
+      && fn->getModule().getStage() != SILStage::Raw) {
     if (OwnershipForwardingMixin::isa(inst))
       if (auto newNext = tryEliminateUnneededForwardingInst(inst, *this))
         return *newNext;

lib/SILOptimizer/Utils/InstOptUtils.cpp

@@ -1792,6 +1792,7 @@ void swift::endLifetimeAtLeakingBlocks(SILValue value,
       });
 }
 
+// TODO: this currently fails to notify the pass with notifyNewInstruction.
 void swift::salvageDebugInfo(SILInstruction *I) {
   if (!I)
     return;
@@ -1872,6 +1873,38 @@ void swift::salvageDebugInfo(SILInstruction *I) {
   }
 }
 
+// TODO: this currently fails to notify the pass with notifyNewInstruction.
+void swift::createDebugFragments(SILValue oldValue, Projection proj,
+                                 SILValue newValue) {
+  if (proj.getKind() != ProjectionKind::Struct)
+    return;
+
+  for (auto *use : getDebugUses(oldValue)) {
+    auto debugVal = dyn_cast<DebugValueInst>(use->getUser());
+    if (!debugVal)
+      continue;
+
+    // Can't create a fragment of a fragment.
+    auto varInfo = debugVal->getVarInfo();
+    if (!varInfo || varInfo->DIExpr.hasFragment())
+      continue;
+
+    SILType baseType = oldValue->getType();
+
+    // Copy VarInfo and add the corresponding fragment DIExpression.
+    SILDebugVariable newVarInfo = *varInfo;
+    newVarInfo.DIExpr.append(
+        SILDebugInfoExpression::createFragment(proj.getVarDecl(baseType)));
+
+    if (!newVarInfo.Type)
+      newVarInfo.Type = baseType;
+
+    // Create a new debug_value
+    SILBuilder(debugVal, debugVal->getDebugScope())
+        .createDebugValue(debugVal->getLoc(), newValue, newVarInfo);
+  }
+}
+
 IntegerLiteralInst *swift::optimizeBuiltinCanBeObjCClass(BuiltinInst *bi,
                                                          SILBuilder &builder) {
   assert(bi->getBuiltinInfo().ID == BuiltinValueKind::CanBeObjCClass);

lib/SILOptimizer/Utils/InstructionDeleter.cpp

@@ -19,13 +19,13 @@
 using namespace swift;
 
 static bool hasOnlyIncidentalUses(SILInstruction *inst,
-                                  bool disallowDebugUses = false) {
+                                  bool preserveDebugInfo = false) {
   for (SILValue result : inst->getResults()) {
     for (Operand *use : result->getUses()) {
       SILInstruction *user = use->getUser();
       if (!isIncidentalUse(user))
         return false;
-      if (disallowDebugUses && user->isDebugInstruction())
+      if (preserveDebugInfo && user->isDebugInstruction())
         return false;
     }
   }
@@ -161,9 +161,9 @@ bool InstructionDeleter::trackIfDead(SILInstruction *inst) {
 }
 
 void InstructionDeleter::forceTrackAsDead(SILInstruction *inst) {
-  bool disallowDebugUses = inst->getFunction()->getEffectiveOptimizationMode()
+  bool preserveDebugInfo = inst->getFunction()->getEffectiveOptimizationMode()
                            <= OptimizationMode::NoOptimization;
-  assert(hasOnlyIncidentalUses(inst, disallowDebugUses));
+  assert(hasOnlyIncidentalUses(inst, preserveDebugInfo));
   getCallbacks().notifyWillBeDeleted(inst);
   deadInstructions.insert(inst);
 }
@@ -274,17 +274,16 @@ bool InstructionDeleter::deleteIfDead(SILInstruction *inst) {
 
 void InstructionDeleter::forceDeleteAndFixLifetimes(SILInstruction *inst) {
   SILFunction *fun = inst->getFunction();
-  bool disallowDebugUses =
+  bool preserveDebugInfo =
       fun->getEffectiveOptimizationMode() <= OptimizationMode::NoOptimization;
-  assert(hasOnlyIncidentalUses(inst, disallowDebugUses));
+  assert(hasOnlyIncidentalUses(inst, preserveDebugInfo));
   deleteWithUses(inst, /*fixLifetimes*/ fun->hasOwnership());
 }
 
 void InstructionDeleter::forceDelete(SILInstruction *inst) {
-  bool disallowDebugUses =
-      inst->getFunction()->getEffectiveOptimizationMode() <=
-      OptimizationMode::NoOptimization;
-  assert(hasOnlyIncidentalUses(inst, disallowDebugUses));
+  bool preserveDebugInfo = inst->getFunction()->getEffectiveOptimizationMode()
+                           <= OptimizationMode::NoOptimization;
+  assert(hasOnlyIncidentalUses(inst, preserveDebugInfo));
   deleteWithUses(inst, /*fixLifetimes*/ false);
 }
 

test/DebugInfo/optimizer_pipeline.swift

@@ -0,0 +1,21 @@
+// RUN: %target-swift-frontend %s -emit-sil -g -Osize -parse-stdlib -parse-as-library -enable-ossa-modules -o - | %FileCheck %s
+
+// REQUIRES: asserts
+
+import Swift
+
+// Test that DCE correctly preserves debug locations.
+// SR-15300: Compiler crash when using Builtin.unreachable in initializers
+//
+// CHECK: sil_scope [[S1:[0-9]+]] { {{.*}} parent @$s18optimizer_pipeline1AVyACs13KeyValuePairsVyypypGcfC
+// CHECK: sil_scope [[S2:[0-9]+]] { {{.*}} parent [[S1]] }
+//
+// CHECK-LABEL: sil @$s18optimizer_pipeline1AVyACs13KeyValuePairsVyypypGcfC : $@convention(method) (@owned KeyValuePairs<Any, Any>, @thin A.Type) -> A {
+// CHECK: bb0(%0 : $KeyValuePairs<Any, Any>, %1 : $@thin A.Type):
+// CHECK: unreachable , scope [[S2]]
+// CHECK-LABEL: } // end sil function '$s18optimizer_pipeline1AVyACs13KeyValuePairsVyypypGcfC'
+public struct A {
+  public init(_ pairs: KeyValuePairs<Any, Any>) {
+    Builtin.unreachable()
+  }
+}

test/SIL/crash-sr-15300.swift

@@ -4,7 +4,7 @@
 // Run the (mandatory) passes on which constant evaluator depends, and test the
 // constant evaluator on the SIL produced after the dependent passes are run.
 //
-// RUN: not %target-sil-opt -silgen-cleanup -raw-sil-inst-lowering -allocbox-to-stack -mandatory-inlining -constexpr-limit 3000 -test-constant-evaluable-subset %t/constant_evaluable_subset_test_silgen.sil > %t/constant_evaluable_subset_test.sil 2> %t/error-output
+// RUN: not %target-sil-opt -opt-mode=speed -silgen-cleanup -raw-sil-inst-lowering -allocbox-to-stack -mandatory-inlining -constexpr-limit 3000 -test-constant-evaluable-subset %t/constant_evaluable_subset_test_silgen.sil > %t/constant_evaluable_subset_test.sil 2> %t/error-output
 //
 // RUN: %FileCheck %S/Inputs/constant_evaluable.swift < %t/error-output
 //
@@ -14,6 +14,6 @@
 // especially performance inlining as it inlines functions such as String.+=
 // that the evaluator has special knowledge about.
 //
-// RUN: not %target-sil-opt -silgen-cleanup -diagnose-invalid-escaping-captures -diagnose-static-exclusivity -capture-promotion -access-enforcement-selection -allocbox-to-stack -noreturn-folding -definite-init -raw-sil-inst-lowering -closure-lifetime-fixup -semantic-arc-opts -destroy-hoisting -ownership-model-eliminator -mandatory-inlining -predictable-memaccess-opts -os-log-optimization -diagnostic-constant-propagation -predictable-deadalloc-elim -mandatory-arc-opts -diagnose-unreachable -diagnose-infinite-recursion -yield-once-check -dataflow-diagnostics -split-non-cond_br-critical-edges -constexpr-limit 3000 -test-constant-evaluable-subset %t/constant_evaluable_subset_test_silgen.sil > /dev/null 2> %t/error-output-mandatory
+// RUN: not %target-sil-opt -opt-mode=speed -silgen-cleanup -diagnose-invalid-escaping-captures -diagnose-static-exclusivity -capture-promotion -access-enforcement-selection -allocbox-to-stack -noreturn-folding -definite-init -raw-sil-inst-lowering -closure-lifetime-fixup -semantic-arc-opts -destroy-hoisting -ownership-model-eliminator -mandatory-inlining -predictable-memaccess-opts -os-log-optimization -diagnostic-constant-propagation -predictable-deadalloc-elim -mandatory-arc-opts -diagnose-unreachable -diagnose-infinite-recursion -yield-once-check -dataflow-diagnostics -split-non-cond_br-critical-edges -constexpr-limit 3000 -test-constant-evaluable-subset %t/constant_evaluable_subset_test_silgen.sil > /dev/null 2> %t/error-output-mandatory
 //
 // RUN: %FileCheck %S/Inputs/constant_evaluable.swift < %t/error-output-mandatory