Skip to content

[region-isolation] Fixes that enable the stdlib to build with region based isolation #71429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 33 additions & 7 deletions include/swift/AST/DiagnosticsSIL.def
Original file line number Diff line number Diff line change
Expand Up @@ -894,8 +894,20 @@ NOTE(sil_referencebinding_inout_binding_here, none,
"inout binding here",
())

// Warnings arising from the flow-sensitive checking of Sendability of
// non-Sendable values
//===----------------------------------------------------------------------===//
// MARK: Region Based Isolation Diagnostics
//===----------------------------------------------------------------------===//

NOTE(regionbasedisolation_maybe_race, none,
"access here could race", ())
ERROR(regionbasedisolation_unknown_pattern, none,
"pattern that the region based isolation checker does not understand how to check. Please file a bug",
())

//===---
// Old Transfer Non Sendable Diagnostics
//

ERROR(regionbasedisolation_selforargtransferred, none,
"call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller", ())
ERROR(regionbasedisolation_transfer_yields_race_no_isolation, none,
Expand All @@ -919,14 +931,28 @@ ERROR(regionbasedisolation_arg_transferred, none,
ERROR(regionbasedisolation_arg_passed_to_strongly_transferred_param, none,
"task isolated value of type %0 passed as a strongly transferred parameter; later accesses could race",
(Type))
NOTE(regionbasedisolation_maybe_race, none,
"access here could race", ())
NOTE(regionbasedisolation_isolated_since_in_same_region_basename, none,
"value is %0 since it is in the same region as %1",
(StringRef, DeclBaseName))
ERROR(regionbasedisolation_unknown_pattern, none,
"pattern that the region based isolation checker does not understand how to check. Please file a bug",
())

//===---
// New Transfer Non Sendable Diagnostics
//

ERROR(regionbasedisolation_named_transfer_yields_race, none,
"transferring non-Sendable value %0 could yield races with later accesses",
(Identifier))
NOTE(regionbasedisolation_named_info_transfer_yields_race, none,
"%0 is transferred from %1 caller to %2 callee. Later uses in caller could race with potential uses in callee",
(Identifier, ActorIsolation, ActorIsolation))
NOTE(regionbasedisolation_named_info_isolated_capture, none,
"%1 value %0 is captured by %2 closure. Later local uses could race",
(Identifier, ActorIsolation, ActorIsolation))
NOTE(regionbasedisolation_named_arg_info, none,
"Transferring task isolated function argument %0 could yield races with caller uses",
(Identifier))
NOTE(regionbasedisolation_named_stronglytransferred_binding, none,
"Cannot access a transferring parameter after the parameter has been transferred", ())

// TODO: print the name of the nominal type
ERROR(deinit_not_visible, none,
Expand Down
13 changes: 13 additions & 0 deletions include/swift/SIL/DebugUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,19 @@ struct DebugVarCarryingInst : VarDeclCarryingInst {
return varName;
}

std::optional<StringRef> maybeGetName() const {
assert(getKind() != Kind::Invalid);
if (auto varInfo = getVarInfo()) {
return varInfo->Name;
}

if (auto *decl = getDecl()) {
return decl->getBaseName().userFacingName();
}

return {};
}

/// Take in \p inst, a potentially invalid DebugVarCarryingInst, and returns a
/// name for it. If we have an invalid value or don't find var info or a decl,
/// return "unknown".
Expand Down
116 changes: 116 additions & 0 deletions include/swift/SILOptimizer/Utils/VariableNameUtils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
//===--- VariableNameUtils.h ----------------------------------------------===//
//
// 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
//
//===----------------------------------------------------------------------===//
///
/// Utilities for inferring the name of a value.
///
//===----------------------------------------------------------------------===//

#ifndef SWIFT_SILOPTIMIZER_UTILS_VARIABLENAMEUTILS_H
#define SWIFT_SILOPTIMIZER_UTILS_VARIABLENAMEUTILS_H

#include "swift/SIL/ApplySite.h"
#include "swift/SIL/DebugUtils.h"
#include "swift/SIL/MemAccessUtils.h"
#include "swift/SIL/SILInstruction.h"
#include "swift/SIL/StackList.h"

namespace swift {

class VariableNameInferrer {
/// The stacklist that we use to process from use->
StackList<PointerUnion<SILInstruction *, SILValue>> variableNamePath;

/// The root value of our string.
///
/// If set, a diagnostic should do a 'root' is defined here error.
SILValue rootValue;

/// The final string we computed.
SmallString<64> &resultingString;

public:
VariableNameInferrer(SILFunction *fn, SmallString<64> &resultingString)
: variableNamePath(fn), resultingString(resultingString) {}

/// Attempts to infer a name from just uses of \p searchValue.
///
/// Returns true if we found a name.
bool tryInferNameFromUses(SILValue searchValue) {
auto *use = getAnyDebugUse(searchValue);
if (!use)
return false;

auto debugVar = DebugVarCarryingInst(use->getUser());
if (!debugVar)
return false;

assert(debugVar.getKind() == DebugVarCarryingInst::Kind::DebugValue);
resultingString += debugVar.getName();
return true;
}

/// See if \p searchValue or one of its defs (walking use->def) has a name
/// that we can use.
///
/// \p failInsteadOfEmittingUnknown set to true if we should return false
/// rather than emitting unknown (and always succeeding).
///
/// \returns true if we inferred anything. Returns false otherwise.
bool inferByWalkingUsesToDefs(SILValue searchValue) {
// Look up our root value while adding to the variable name path list.
auto rootValue = findDebugInfoProvidingValue(searchValue);
if (!rootValue) {
// If we do not pattern match successfully, just set resulting string to
// unknown and return early.
resultingString += "unknown";
return true;
}

drainVariableNamePath();
return true;
}

/// Infers the value that provides the debug info. This can be something like
/// an alloc_stack that provides the information directly or a value that has
/// a debug_value as a user.
///
/// \returns SILValue() if we did not find anything.
SILValue inferByWalkingUsesToDefsReturningRoot(SILValue searchValue) {
// Look up our root value while adding to the variable name path list.
auto rootValue = findDebugInfoProvidingValue(searchValue);
if (!rootValue) {
// If we do not pattern match successfully, return SILValue() early.
return SILValue();
}

drainVariableNamePath();
return rootValue;
}

StringRef getName() const { return resultingString; }

private:
void drainVariableNamePath();

/// Finds the SILValue that either provides the direct debug information or
/// that has a debug_value user that provides the name of the value.
SILValue findDebugInfoProvidingValue(SILValue searchValue);

/// Given an initialized once allocation inst without a ValueDecl or a
/// DebugVariable provided name, attempt to find a root value from its
/// initialization.
SILValue getRootValueForTemporaryAllocation(AllocationInst *allocInst);
};

} // namespace swift

#endif
34 changes: 25 additions & 9 deletions lib/SILOptimizer/Analysis/RegionAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,17 @@ struct UseDefChainVisitor
// do not want to treat this as a merge.
if (auto p = Projection(inst)) {
switch (p.getKind()) {
// Currently if we load and then project_box from a memory location,
// we treat that as a projection. This follows the semantics/notes in
// getAccessProjectionOperand.
case ProjectionKind::Box:
return cast<ProjectBoxInst>(inst)->getOperand();
case ProjectionKind::Upcast:
case ProjectionKind::RefCast:
case ProjectionKind::BlockStorageCast:
case ProjectionKind::BitwiseCast:
case ProjectionKind::TailElems:
case ProjectionKind::Box:
case ProjectionKind::Class:
case ProjectionKind::TailElems:
llvm_unreachable("Shouldn't see this here");
case ProjectionKind::Index:
// Index is always a merge.
Expand Down Expand Up @@ -1289,8 +1293,7 @@ class PartitionOpTranslator {
continue;
}
if (auto *pbi = dyn_cast<ProjectBoxInst>(val)) {
if (isNonSendableType(
pbi->getType().getSILBoxFieldType(function))) {
if (isNonSendableType(pbi->getType())) {
auto trackVal = getTrackableValue(val, true);
(void)trackVal;
continue;
Expand Down Expand Up @@ -1431,8 +1434,12 @@ class PartitionOpTranslator {

if (auto tryApplyInst = dyn_cast<TryApplyInst>(inst)) {
foundResults.emplace_back(tryApplyInst->getNormalBB()->getArgument(0));
if (tryApplyInst->getErrorBB()->getNumArguments() > 0)
if (tryApplyInst->getErrorBB()->getNumArguments() > 0) {
foundResults.emplace_back(tryApplyInst->getErrorBB()->getArgument(0));
}
for (auto indirectResults : tryApplyInst->getIndirectSILResults()) {
foundResults.emplace_back(indirectResults);
}
return;
}

Expand Down Expand Up @@ -1797,10 +1804,10 @@ class PartitionOpTranslator {
};

if (applySite.hasSelfArgument()) {
handleSILOperands(applySite.getOperandsWithoutSelf());
handleSILOperands(applySite.getOperandsWithoutIndirectResultsOrSelf());
handleSILSelf(&applySite.getSelfArgumentOperand());
} else {
handleSILOperands(applySite->getAllOperands());
handleSILOperands(applySite.getOperandsWithoutIndirectResults());
}

// non-sendable results can't be returned from cross-isolation calls without
Expand Down Expand Up @@ -2109,6 +2116,7 @@ class PartitionOpTranslator {
}

case TranslationSemantics::Asserting:
llvm::errs() << "BannedInst: " << *inst;
llvm::report_fatal_error(
"transfer-non-sendable: Found banned instruction?!");
return;
Expand All @@ -2119,6 +2127,7 @@ class PartitionOpTranslator {
return ::isNonSendableType(value->getType(), inst->getFunction());
}))
return;
llvm::errs() << "BadInst: " << *inst;
llvm::report_fatal_error(
"transfer-non-sendable: Found instruction that is not allowed to "
"have non-Sendable parameters with such parameters?!");
Expand Down Expand Up @@ -2304,7 +2313,6 @@ CONSTANT_TRANSLATION(TupleInst, Assign)
CONSTANT_TRANSLATION(BeginAccessInst, LookThrough)
CONSTANT_TRANSLATION(BeginBorrowInst, LookThrough)
CONSTANT_TRANSLATION(BeginDeallocRefInst, LookThrough)
CONSTANT_TRANSLATION(RefToBridgeObjectInst, LookThrough)
CONSTANT_TRANSLATION(BridgeObjectToRefInst, LookThrough)
CONSTANT_TRANSLATION(CopyValueInst, LookThrough)
CONSTANT_TRANSLATION(ExplicitCopyValueInst, LookThrough)
Expand Down Expand Up @@ -2414,6 +2422,8 @@ CONSTANT_TRANSLATION(BeginUnpairedAccessInst, Require)
CONSTANT_TRANSLATION(ValueMetatypeInst, Require)
// Require of the value we extract the metatype from.
CONSTANT_TRANSLATION(ExistentialMetatypeInst, Require)
// These can take a parameter. If it is non-Sendable, use a require.
CONSTANT_TRANSLATION(GetAsyncContinuationAddrInst, Require)

//===---
// Asserting If Non Sendable Parameter
Expand Down Expand Up @@ -2454,7 +2464,6 @@ CONSTANT_TRANSLATION(DynamicMethodBranchInst, TerminatorPhi)
// (UnsafeContinuation and UnsafeThrowingContinuation).
CONSTANT_TRANSLATION(AwaitAsyncContinuationInst, AssertingIfNonSendable)
CONSTANT_TRANSLATION(GetAsyncContinuationInst, AssertingIfNonSendable)
CONSTANT_TRANSLATION(GetAsyncContinuationAddrInst, AssertingIfNonSendable)
CONSTANT_TRANSLATION(ExtractExecutorInst, AssertingIfNonSendable)

//===---
Expand Down Expand Up @@ -2614,6 +2623,13 @@ CAST_WITH_MAYBE_SENDABLE_NONSENDABLE_OP_AND_RESULT(UncheckedValueCastInst)
// Custom Handling
//

TranslationSemantics
PartitionOpTranslator::visitRefToBridgeObjectInst(RefToBridgeObjectInst *r) {
translateSILLookThrough(
SILValue(r), r->getOperand(RefToBridgeObjectInst::ConvertedOperand));
return TranslationSemantics::Special;
}

TranslationSemantics
PartitionOpTranslator::visitPackElementGetInst(PackElementGetInst *r) {
if (!isNonSendableType(r->getType()))
Expand Down
Loading