Merge pull request #73864 from Snowy1803/scs-builder-skip-meta

Skip meta instructions for Builder.init with automatic location

Author: Snowy1803

SwiftCompilerSources/Sources/Optimizer/PassManager/Context.swift

@@ -433,6 +433,34 @@ extension Type {
 //                          Builder initialization
 //===----------------------------------------------------------------------===//
 
+private extension Instruction {
+  /// Returns self, unless self is a meta instruction, in which case the next
+  /// non-meta instruction is returned. Returns nil if there are no non-meta
+  /// instructions in the basic block.
+  var nextNonMetaInstruction: Instruction? {
+    for inst in InstructionList(first: self) where !(inst is MetaInstruction) {
+      return inst
+    }
+    return nil
+  }
+
+  /// Returns the next interesting location. As it is impossible to set a
+  /// breakpoint on a meta instruction, those are skipped.
+  /// However, we don't want to take a location with different inlining
+  /// information than this instruction, so in that case, we will return the
+  /// location of the meta instruction. If the meta instruction is the only
+  /// instruction in the basic block, we also take its location.
+  var locationOfNextNonMetaInstruction: Location {
+    let location = self.location
+    guard !location.isInlined,
+          let nextLocation = nextNonMetaInstruction?.location,
+          !nextLocation.isInlined else {
+      return location
+    }
+    return nextLocation
+  }
+}
+
 extension Builder {
   /// Creates a builder which inserts _before_ `insPnt`, using a custom `location`.
   init(before insPnt: Instruction, location: Location, _ context: some MutatingContext) {
@@ -441,8 +469,23 @@ extension Builder {
               context.notifyInstructionChanged, context._bridged.asNotificationHandler())
   }
 
-  /// Creates a builder which inserts _before_ `insPnt`, using the location of `insPnt`.
+  /// Creates a builder which inserts before `insPnt`, using `insPnt`'s next
+  /// non-meta instruction's location.
+  /// This function should be used when moving code to an unknown insert point,
+  /// when we want to inherit the location of the closest non-meta instruction.
+  /// For replacing an existing meta instruction with another, use
+  /// ``Builder.init(replacing:_:)``.
   init(before insPnt: Instruction, _ context: some MutatingContext) {
+    context.verifyIsTransforming(function: insPnt.parentFunction)
+    self.init(insertAt: .before(insPnt),
+              location: insPnt.locationOfNextNonMetaInstruction,
+              context.notifyInstructionChanged, context._bridged.asNotificationHandler())
+  }
+
+  /// Creates a builder which inserts _before_ `insPnt`, using the exact location of `insPnt`,
+  /// for the purpose of replacing that meta instruction with an equivalent instruction.
+  /// This function does not delete `insPnt`.
+  init(replacing insPnt: MetaInstruction, _ context: some MutatingContext) {
     context.verifyIsTransforming(function: insPnt.parentFunction)
     self.init(insertAt: .before(insPnt), location: insPnt.location,
               context.notifyInstructionChanged, context._bridged.asNotificationHandler())
@@ -460,10 +503,10 @@ extension Builder {
     }
   }
 
-  /// Creates a builder which inserts _after_ `insPnt`, using the location of `insPnt`.
+  /// Creates a builder which inserts _after_ `insPnt`, using `insPnt`'s next
+  /// non-meta instruction's location.
   init(after insPnt: Instruction, _ context: some MutatingContext) {
-    context.verifyIsTransforming(function: insPnt.parentFunction)
-    self.init(after: insPnt, location: insPnt.location, context)
+    self.init(after: insPnt, location: insPnt.locationOfNextNonMetaInstruction, context)
   }
 
   /// Creates a builder which inserts at the end of `block`, using a custom `location`.
@@ -482,11 +525,12 @@ extension Builder {
   }
 
   /// Creates a builder which inserts at the begin of `block`, using the location of the first
-  /// instruction of `block`.
+  /// non-meta instruction of `block`.
   init(atBeginOf block: BasicBlock, _ context: some MutatingContext) {
     context.verifyIsTransforming(function: block.parentFunction)
     let firstInst = block.instructions.first!
-    self.init(insertAt: .before(firstInst), location: firstInst.location,
+    self.init(insertAt: .before(firstInst),
+              location: firstInst.locationOfNextNonMetaInstruction,
               context.notifyInstructionChanged, context._bridged.asNotificationHandler())
   }
 

SwiftCompilerSources/Sources/SIL/Instruction.swift

@@ -418,7 +418,16 @@ public protocol DebugVariableInstruction : VarDeclInstruction {
   var debugVariable: DebugVariable { get }
 }
 
-final public class DebugValueInst : Instruction, UnaryInstruction, DebugVariableInstruction {
+/// A meta instruction is an instruction whose location is not interesting as
+/// it is impossible to set a breakpoint on it.
+/// That could be because the instruction does not generate code (such as
+/// `debug_value`), or because the generated code would be in the prologue
+/// (`alloc_stack`).
+/// When we are moving code onto an unknown instruction (such as the start of a
+/// basic block), we want to ignore any meta instruction that might be there.
+public protocol MetaInstruction: Instruction {}
+
+final public class DebugValueInst : Instruction, UnaryInstruction, DebugVariableInstruction, MetaInstruction {
   public var varDecl: VarDecl? {
     VarDecl(bridged: bridged.DebugValue_getDecl())
   }
@@ -1127,7 +1136,7 @@ final public class InitBlockStorageHeaderInst: SingleValueInstruction {}
 
 public protocol Allocation : SingleValueInstruction { }
 
-final public class AllocStackInst : SingleValueInstruction, Allocation, DebugVariableInstruction {
+final public class AllocStackInst : SingleValueInstruction, Allocation, DebugVariableInstruction, MetaInstruction {
   public var hasDynamicLifetime: Bool { bridged.AllocStackInst_hasDynamicLifetime() }
 
   public var varDecl: VarDecl? {

SwiftCompilerSources/Sources/SIL/Location.swift

@@ -30,6 +30,7 @@ public struct Location: Equatable, CustomStringConvertible {
 
   public var hasValidLineNumber: Bool { bridged.hasValidLineNumber() }
   public var isAutoGenerated: Bool { bridged.isAutoGenerated() }
+  public var isInlined: Bool { bridged.isInlined() }
 
   public var isDebugSteppable: Bool { hasValidLineNumber && !isAutoGenerated }
 

include/swift/SIL/SILBridging.h

@@ -534,6 +534,7 @@ struct BridgedLocation {
   SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedLocation getAutogeneratedLocation() const;
   BRIDGED_INLINE bool hasValidLineNumber() const;
   BRIDGED_INLINE bool isAutoGenerated() const;
+  BRIDGED_INLINE bool isInlined() const;
   BRIDGED_INLINE bool isEqualTo(BridgedLocation rhs) const;
   SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedSourceLoc getSourceLocation() const;
   BRIDGED_INLINE bool hasSameSourceLocation(BridgedLocation rhs) const;

include/swift/SIL/SILBridgingImpl.h

@@ -566,6 +566,9 @@ bool BridgedLocation::hasValidLineNumber() const {
 bool BridgedLocation::isAutoGenerated() const {
   return getLoc().isAutoGenerated();
 }
+bool BridgedLocation::isInlined() const {
+  return getLoc().getScope()->InlinedCallSite;
+}
 bool BridgedLocation::isEqualTo(BridgedLocation rhs) const {
   return getLoc().isEqualTo(rhs.getLoc());
 }

test/DebugInfo/simplify_checked_cast_br.sil

@@ -0,0 +1,39 @@
+// RUN: %target-sil-opt -enable-sil-verify-all %s -simplification -simplify-instruction=checked_cast_br -sil-print-debuginfo | %FileCheck %s
+
+// REQUIRES: swift_in_compiler
+
+import Swift
+import Builtin
+
+protocol P : AnyObject {}
+class B : P {}
+class X : B {}
+
+// CHECK-LABEL: sil [ossa] @test_ossa :
+// CHECK:         %0 = alloc_ref
+// CHECK-NEXT:  checked_cast_br AnyObject in %0 : $X
+// CHECK:     bb2([[A:%.*]] : @owned $X):
+// CHECK-NEXT:  [[U:%.*]] = upcast [[A]] : $X to $B, loc "takethis":1:1
+// CHECK-NEXT:  [[E:%.*]] = init_existential_ref [[U]] {{.+}}, loc "takethis":1:1
+// CHECK-NEXT:  debug_value [[E]] : $AnyObject, let, name "me", loc "ignore-this-meta-inst":1:1
+// CHECK-NEXT:  destroy_value [[E]] : $AnyObject, loc "takethis":1:1
+// CHECK:       } // end sil function 'test_ossa'
+sil [ossa] @test_ossa : $@convention(thin) () -> @owned X {
+bb0:
+  %0 = alloc_ref $X
+  %1 = upcast %0 : $X to $B
+  %2 = init_existential_ref %1 : $B : $B, $AnyObject
+  debug_value %2 : $AnyObject, name "x"
+  checked_cast_br AnyObject in %2 : $AnyObject to X, bb1, bb2
+
+bb1(%5 : @owned $X):
+  return %5 : $X
+
+bb2(%7 : @owned $AnyObject):
+  debug_value %7 : $AnyObject, let, name "me", loc "ignore-this-meta-inst":1:1
+  destroy_value %7: $AnyObject, loc "takethis":1:1
+  unreachable
+}
+
+sil_vtable X {
+}