From 7eb2cb82e4613dd90d9859a63ae4d6fb6d7959f6 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Wed, 8 Feb 2023 19:11:32 +0100 Subject: [PATCH] Swift Optimizer: add a pass to cleanup debug_step instructions If a `debug_step` has the same debug location as a previous or succeeding instruction it is removed. It's just important that there is at least one instruction for a certain debug location so that single stepping on that location will work. --- .../Optimizer/FunctionPasses/CMakeLists.txt | 1 + .../FunctionPasses/CleanupDebugSteps.swift | 83 ++++++++++ .../PassManager/PassRegistration.swift | 1 + .../swift/SILOptimizer/PassManager/Passes.def | 2 + test/SILOptimizer/cleanup_debugstep.sil | 154 ++++++++++++++++++ 5 files changed, 241 insertions(+) create mode 100644 SwiftCompilerSources/Sources/Optimizer/FunctionPasses/CleanupDebugSteps.swift create mode 100644 test/SILOptimizer/cleanup_debugstep.sil diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/CMakeLists.txt b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/CMakeLists.txt index f60a5bf23d3..bf43f0302f5 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/CMakeLists.txt +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/CMakeLists.txt @@ -8,6 +8,7 @@ swift_compiler_sources(Optimizer AssumeSingleThreaded.swift + CleanupDebugSteps.swift ComputeEscapeEffects.swift ComputeSideEffects.swift ObjCBridgingOptimization.swift diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/CleanupDebugSteps.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/CleanupDebugSteps.swift new file mode 100644 index 00000000000..469e7e9af61 --- /dev/null +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/CleanupDebugSteps.swift @@ -0,0 +1,83 @@ +//===--- SimplificationPasses.swift ----------------------------------------==// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import SIL + +/// Removes redundant `debug_step` instructions. +/// If a `debug_step` has the same debug location as a previous or succeeding instruction +/// it is removed. It's just important that there is at least one instruction for a +/// certain debug location so that single stepping on that location will work. +let cleanupDebugStepsPass = FunctionPass(name: "cleanup-debug-steps") { + (function: Function, context: FunctionPassContext) in + + for block in function.blocks { + cleanupDebugSteps(in: block, context) + } +} + +private func cleanupDebugSteps(in block: BasicBlock, _ context: FunctionPassContext) { + var lastInstWithSameLocation: Instruction? + + for inst in block.instructions { + if !inst.location.isDebugSteppable { + if inst is DebugStepInst && !inst.location.isDebugSteppable { + // First case: the instruction which is replaced by the debug_step didn't have a valid + // location itself. Then we don't need the debug_step either. + context.erase(instruction: inst) + } + continue + } + + if let li = lastInstWithSameLocation, + !inst.location.hasSameSourceLocation(as: li.location) { + lastInstWithSameLocation = nil + } + + // Only instructions which are really compiled down to some machine instructions can be + // single stepped on. + if !inst.producesMachineCode { + continue + } + + if let li = lastInstWithSameLocation { + if inst is DebugStepInst { + + // Second case: + // %li = some_instruction, loc "l" + // debug_step, loc "l" // current inst -> erase + context.erase(instruction: inst) + continue + } else if li is DebugStepInst { + + // Third case: + // debug_step, loc "l" // li -> erase + // %inst = some_instruction, loc "l" // current inst + context.erase(instruction: li) + } + } + lastInstWithSameLocation = inst + } +} + +private extension Instruction { + var producesMachineCode: Bool { + switch self { + // We could include more instructions here. + // In worst case a debug_step instruction remains in the code although it's not needed. + // This is harmless. + case is DebugStepInst, is ApplySite, is LoadInst, is StoreInst, is TermInst: + return location.isDebugSteppable + default: + return false + } + } +} diff --git a/SwiftCompilerSources/Sources/Optimizer/PassManager/PassRegistration.swift b/SwiftCompilerSources/Sources/Optimizer/PassManager/PassRegistration.swift index 69cb6d5813c..c7290e6a1b4 100644 --- a/SwiftCompilerSources/Sources/Optimizer/PassManager/PassRegistration.swift +++ b/SwiftCompilerSources/Sources/Optimizer/PassManager/PassRegistration.swift @@ -75,6 +75,7 @@ private func registerSwiftPasses() { registerPass(simplificationPass, { simplificationPass.run($0) }) registerPass(ononeSimplificationPass, { ononeSimplificationPass.run($0) }) registerPass(lateOnoneSimplificationPass, { lateOnoneSimplificationPass.run($0) }) + registerPass(cleanupDebugStepsPass, { cleanupDebugStepsPass.run($0) }) // Instruction passes registerForSILCombine(BeginCOWMutationInst.self, { run(BeginCOWMutationInst.self, $0) }) diff --git a/include/swift/SILOptimizer/PassManager/Passes.def b/include/swift/SILOptimizer/PassManager/Passes.def index a2a1be0cdc4..f4fe5495961 100644 --- a/include/swift/SILOptimizer/PassManager/Passes.def +++ b/include/swift/SILOptimizer/PassManager/Passes.def @@ -395,6 +395,8 @@ SWIFT_FUNCTION_PASS(OnoneSimplification, "onone-simplification", "Peephole simplifications which runs at -Onone") SWIFT_FUNCTION_PASS(LateOnoneSimplification, "late-onone-simplification", "Peephole simplifications which can only run late in the -Onone pipeline") +SWIFT_FUNCTION_PASS(CleanupDebugSteps, "cleanup-debug-steps", + "Cleanup debug_step instructions for Onone") PASS(SimplifyBBArgs, "simplify-bb-args", "SIL Block Argument Simplification") PASS(SimplifyCFG, "simplify-cfg", diff --git a/test/SILOptimizer/cleanup_debugstep.sil b/test/SILOptimizer/cleanup_debugstep.sil new file mode 100644 index 00000000000..f6f947065dd --- /dev/null +++ b/test/SILOptimizer/cleanup_debugstep.sil @@ -0,0 +1,154 @@ +// RUN: %target-sil-opt -enable-sil-verify-all %s -cleanup-debug-steps -sil-print-debuginfo | %FileCheck %s + +// REQUIRES: swift_in_compiler + +import Swift +import Builtin + +sil @f : $@convention(thin) () -> () + +// CHECK-LABEL: sil @remove_after_previous_location +// CHECK-NOT: debug_step +// CHECK: } // end sil function 'remove_after_previous_location' +sil @remove_after_previous_location : $@convention(thin) () -> () { +bb0: + %f = function_ref @f : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> (), loc "a.swift":1:2 + debug_step, loc "a.swift":1:2 + %r = tuple () + return %r : $() +} + +// CHECK-LABEL: sil @dont_remove_after_previous_location +// CHECK: debug_step +// CHECK: } // end sil function 'dont_remove_after_previous_location' +sil @dont_remove_after_previous_location : $@convention(thin) () -> () { +bb0: + %f = function_ref @f : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> (), loc "a.swift":1:2 + %r = tuple (), loc "a.swift":27:2 + debug_step, loc "a.swift":1:2 + return %r : $() +} + +// CHECK-LABEL: sil @remove_two_after_previous_location +// CHECK-NOT: debug_step +// CHECK: } // end sil function 'remove_two_after_previous_location' +sil @remove_two_after_previous_location : $@convention(thin) () -> () { +bb0: + %f = function_ref @f : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> (), loc "a.swift":1:2 + debug_step, loc "a.swift":1:2 + debug_step, loc "a.swift":1:2 + %r = tuple () + return %r : $() +} + +// CHECK-LABEL: sil @remove_before_next_location +// CHECK-NOT: debug_step +// CHECK: } // end sil function 'remove_before_next_location' +sil @remove_before_next_location : $@convention(thin) () -> () { +bb0: + %f = function_ref @f : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> () + debug_step, loc "a.swift":1:2 + %r = tuple (), loc "":0:0 + return %r : $(), loc "a.swift":1:2 +} + +// CHECK-LABEL: sil @dont_remove_before_next_location +// CHECK: debug_step +// CHECK: } // end sil function 'dont_remove_before_next_location' +sil @dont_remove_before_next_location : $@convention(thin) () -> () { +bb0: + %f = function_ref @f : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> () + debug_step, loc "a.swift":1:2 + %r = tuple (), loc "a.swift":27:2 + return %r : $(), loc "a.swift":1:2 +} + +// CHECK-LABEL: sil @remove_two_before_next_location +// CHECK-NOT: debug_step +// CHECK: } // end sil function 'remove_two_before_next_location' +sil @remove_two_before_next_location : $@convention(thin) () -> () { +bb0: + %f = function_ref @f : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> () + debug_step, loc "a.swift":1:2 + debug_step, loc "a.swift":1:2 + %r = tuple (), loc "":0:0 + return %r : $(), loc "a.swift":1:2 +} + +// CHECK-LABEL: sil @remove_null_location +// CHECK-NOT: debug_step +// CHECK: } // end sil function 'remove_null_location' +sil @remove_null_location : $@convention(thin) () -> () { +bb0: + %f = function_ref @f : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> () + debug_step, loc "":0:0 + %r = tuple () + return %r : $(), loc "a.swift":1:2 +} +// CHECK-LABEL: sil @dont_remove_one +// CHECK: debug_step +// CHECK: } // end sil function 'dont_remove_one' +sil @dont_remove_one : $@convention(thin) () -> () { +bb0: + %f = function_ref @f : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> () + debug_step, loc "a.swift":1:2 + %r = tuple () + return %r : $() +} + +// CHECK-LABEL: sil @replace_three_with_one +// CHECK: apply +// CHECK-NEXT: debug_step , loc "a.swift":1:2 +// CHECK-NEXT: tuple +// CHECK: } // end sil function 'replace_three_with_one' +sil @replace_three_with_one : $@convention(thin) () -> () { +bb0: + %f = function_ref @f : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> () + debug_step, loc "a.swift":1:2 + debug_step, loc "a.swift":1:2 + debug_step, loc "a.swift":1:2 + %r = tuple () + return %r : $() +} + +// CHECK-LABEL: sil @replace_three_with_two +// CHECK: apply +// CHECK-NEXT: debug_step , loc "a.swift":1:2 +// CHECK-NEXT: debug_step , loc "a.swift":2:3 +// CHECK-NEXT: tuple +// CHECK: } // end sil function 'replace_three_with_two' +sil @replace_three_with_two : $@convention(thin) () -> () { +bb0: + %f = function_ref @f : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> () + debug_step, loc "a.swift":1:2 + debug_step, loc "a.swift":2:3 + debug_step, loc "a.swift":2:3 + %r = tuple () + return %r : $() +} + + +// CHECK-LABEL: sil @dont_remove_two +// CHECK: debug_step +// CHECK-NEXT: debug_step +// CHECK: } // end sil function 'dont_remove_two' +sil @dont_remove_two : $@convention(thin) () -> () { +bb0: + %f = function_ref @f : $@convention(thin) () -> () + %a = apply %f() : $@convention(thin) () -> () + debug_step, loc "a.swift":1:2 + debug_step, loc "a.swift":2:3 + %r = tuple () + return %r : $() +} +