From 39e63b488fed2c714ff94621b251dae7455864c8 Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Wed, 29 Jan 2025 15:03:57 -0800 Subject: [PATCH 1/2] [sil] Make sure that we do not flag ignored_use as dead code when used after a try_apply or begin_apply. --- .../Mandatory/DiagnoseUnreachable.cpp | 5 +- test/SILOptimizer/noreturn_folding.sil | 50 ++++++++++++++++--- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/lib/SILOptimizer/Mandatory/DiagnoseUnreachable.cpp b/lib/SILOptimizer/Mandatory/DiagnoseUnreachable.cpp index 7d477ee52a36f..ea9051d3156ba 100644 --- a/lib/SILOptimizer/Mandatory/DiagnoseUnreachable.cpp +++ b/lib/SILOptimizer/Mandatory/DiagnoseUnreachable.cpp @@ -777,8 +777,9 @@ static bool simplifyBlocksWithCallsToNoReturn(SILBasicBlock &BB, // If we have an ignored use whose operand is our no return call, ignore it. if (auto *i = dyn_cast(currInst)) { - if (auto *svi = dyn_cast(i->getOperand()); - svi && getAsCallToNoReturn(svi)) { + // This handles try_apply, apply, begin_apply. + if (auto *inst = i->getOperand()->getDefiningInstructionOrTerminator(); + inst && inst == noReturnCall) { return false; } } diff --git a/test/SILOptimizer/noreturn_folding.sil b/test/SILOptimizer/noreturn_folding.sil index 7cdbea8f657ad..17cbe138d8268 100644 --- a/test/SILOptimizer/noreturn_folding.sil +++ b/test/SILOptimizer/noreturn_folding.sil @@ -1,12 +1,13 @@ -// RUN: %target-sil-opt -module-name Swift -enable-sil-verify-all -noreturn-folding < %s | %FileCheck %s +// RUN: %target-sil-opt -enable-sil-verify-all -noreturn-folding -verify %s | %FileCheck %s +import Swift import Builtin -enum Never {} - -struct Int64 { - var value: Builtin.Int64 -} +sil @exit : $@convention(thin) (Builtin.Int32) -> Never +sil @returnNever : $@convention(thin) () -> Never +sil @returnNeverThrows : $@convention(thin) () -> (Never, @error Error) +sil @returnNeverCoroutine : $@yield_once @convention(thin) () -> @yields Never +sil @doSomething : $@convention(thin) () -> () // We used to crash on this IR. We would delete "%4" while there is still a // (dead) user "%7" around. @@ -15,7 +16,7 @@ struct Int64 { // CHECK: %[[E:.+]] = function_ref @exit // CHECK: apply %[[E]] // CHECK: unreachable - +// CHECK: } // end sil function 'unreachable_outside_block_user' sil private @unreachable_outside_block_user : $@convention(thin) () -> Int64 { bb0: %0 = integer_literal $Builtin.Int1, -1 @@ -23,7 +24,9 @@ bb0: // function_ref exit %2 = function_ref @exit : $@convention(thin) (Builtin.Int32) -> Never %3 = apply %2(%1) : $@convention(thin) (Builtin.Int32) -> Never + // expected-note @-1 {{a call to a never-returning function}} %4 = integer_literal $Builtin.Int64, 7 + // expected-warning @-1 {{will never be executed}} cond_br %0, bb1, bb2 bb1: @@ -42,4 +45,35 @@ bb3 (%11: $Int64): return %11 : $Int64 } -sil @exit : $@convention(thin) (Builtin.Int32) -> Never +// Make sure we do not emit any error here. +sil @ignore_use_apply : $@convention(thin) () -> () { +bb0: + %0 = function_ref @returnNever : $@convention(thin) () -> Never + %1 = apply %0() : $@convention(thin) () -> Never + ignored_use %1 : $Never + unreachable +} + +// Make sure we do not emit any error here. +sil [ossa] @ignore_use_try_apply : $@convention(thin) () -> () { +bb0: + %0 = function_ref @returnNeverThrows : $@convention(thin) () -> (Never, @error Error) + try_apply %0() : $@convention(thin) () -> (Never, @error Error), normal bb1, error bb2 + +bb1(%2 : $Never): + ignored_use %2 : $Never + unreachable + +bb2(%5 : @owned $Error): + %6 = builtin "unexpectedError"(%5 : $Error) : $() + unreachable +} + +sil [ossa] @ignore_use_begin_apply : $@convention(thin) () -> () { +bb0: + %0 = function_ref @returnNeverCoroutine : $@yield_once @convention(thin) () -> @yields Never + (%1, %2) = begin_apply %0() : $@yield_once @convention(thin) () -> @yields Never + ignored_use %1 + end_apply %2 as $() + unreachable +} From 8c96a8db1b980d9f1cd72882007db4e7f54ab4e2 Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Wed, 29 Jan 2025 15:05:37 -0800 Subject: [PATCH 2/2] [rbi] When finding closure uses in an immediately invoked closure, distinguish in between captures and parameters. Otherwise, when one diagnoses code like the following: ``` Task { { use($0) }(x) } ``` One gets that $0 was captured instead of x. Unfortunately, since function parameters do not have locations associated with them, we do not mark x itself... instead, we mark the closure... which is unfortunate. --- lib/SILOptimizer/Mandatory/SendNonSendable.cpp | 16 +++++++++------- .../transfernonsendable_sending_params.swift | 6 ++++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/SILOptimizer/Mandatory/SendNonSendable.cpp b/lib/SILOptimizer/Mandatory/SendNonSendable.cpp index d5232514197a1..2a2bec71a3f95 100644 --- a/lib/SILOptimizer/Mandatory/SendNonSendable.cpp +++ b/lib/SILOptimizer/Mandatory/SendNonSendable.cpp @@ -294,14 +294,16 @@ findClosureUse(Operand *initialOperand) { // immediately invoked. In such a case, we can emit a better diagnostic in // the called closure. if (auto fas = FullApplySite::isa(op->getUser())) { - if (auto *f = fas.getCalleeFunction(); - f && f->getDeclRef().getClosureExpr()) { - auto *fArg = f->getArgument(fas.getCalleeArgIndex(*op)); - for (auto *use : fArg->getUses()) { - if (visitedOperand.insert(use).second) - worklist.emplace_back(use, fArg); + if (auto *f = fas.getCalleeFunction()) { + auto *fArg = cast( + f->getArgument(fas.getCalleeArgIndex(*op))); + if (fArg->isClosureCapture()) { + for (auto *use : fArg->getUses()) { + if (visitedOperand.insert(use).second) + worklist.emplace_back(use, fArg); + } + continue; } - continue; } } diff --git a/test/Concurrency/transfernonsendable_sending_params.swift b/test/Concurrency/transfernonsendable_sending_params.swift index 88504220097ce..e71857f4f2a87 100644 --- a/test/Concurrency/transfernonsendable_sending_params.swift +++ b/test/Concurrency/transfernonsendable_sending_params.swift @@ -524,6 +524,12 @@ func taskIsolatedCaptureInSendingClosureLiteral(_ x: NonSendableKlass) { }() } + Task { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} + { // expected-note {{closure captures 'x' which is accessible to code in the current task}} + print($0) + }(x) + } + takeClosure { // expected-warning {{passing closure as a 'sending' parameter risks causing data races between code in the current task and concurrent execution of the closure}} print(x) // expected-note {{closure captures 'x' which is accessible to code in the current task}} }