Skip to content

[NoncopyableWrapperElim] Process undef values. #74298

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 8 commits into from
Jun 12, 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
14 changes: 8 additions & 6 deletions include/swift/SIL/SILFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
#include "swift/SIL/SILDeclRef.h"
#include "swift/SIL/SILLinkage.h"
#include "swift/SIL/SILPrintContext.h"
#include "swift/SIL/SILUndef.h"
#include "llvm/ADT/MapVector.h"

namespace swift {

Expand Down Expand Up @@ -337,12 +339,8 @@ class SILFunction

PerformanceConstraints perfConstraints = PerformanceConstraints::None;

/// This is the set of undef values we've created, for uniquing purposes.
///
/// We use a SmallDenseMap since in most functions, we will have only one type
/// of undef if we have any at all. In that case, by staying small we avoid
/// needing a heap allocation.
llvm::SmallDenseMap<SILType, SILUndef *, 1> undefValues;
/// The undefs of each type in the function.
llvm::SmallMapVector<SILType, SILUndef *, 1> undefValues;

/// This is the number of uses of this SILFunction inside the SIL.
/// It does not include references from debug scopes.
Expand Down Expand Up @@ -1637,6 +1635,10 @@ class SILFunction
decl->getLifetimeAnnotation());
}

ArrayRef<std::pair<SILType, SILUndef *>> getUndefValues() {
return {undefValues.begin(), undefValues.end()};
}

/// verify - Run the SIL verifier to make sure that the SILFunction follows
/// invariants.
void verify(CalleeCache *calleeCache = nullptr,
Expand Down
2 changes: 1 addition & 1 deletion include/swift/SIL/SILInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -9160,7 +9160,7 @@ class MoveOnlyWrapperToCopyableBoxInst
ValueOwnershipKind forwardingOwnershipKind)
: UnaryInstructionBase(
DebugLoc, operand,
operand->getType().removingMoveOnlyWrapperToBoxedType(
operand->getType().removingMoveOnlyWrapperFromBoxedType(
operand->getFunction()),
forwardingOwnershipKind) {
assert(
Expand Down
10 changes: 9 additions & 1 deletion include/swift/SIL/SILType.h
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,15 @@ class SILType {
///
/// DISCUSSION: This is separate from removingMoveOnlyWrapper since this API
/// requires a SILFunction * and is specialized.
SILType removingMoveOnlyWrapperToBoxedType(const SILFunction *fn);
SILType removingMoveOnlyWrapperFromBoxedType(const SILFunction *fn);

/// Whether there's a direct wrapper or a wrapper inside a box.
bool hasAnyMoveOnlyWrapping(const SILFunction *fn) {
return isMoveOnlyWrapped() || isBoxedMoveOnlyWrappedType(fn);
}

/// Removes a direct wrapper from a type or a wrapper from a type in a box.
SILType removingAnyMoveOnlyWrapping(const SILFunction *fn);

/// Returns a SILType with any archetypes mapped out of context.
SILType mapTypeOutOfContext() const;
Expand Down
9 changes: 2 additions & 7 deletions include/swift/SIL/SILValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -599,14 +599,9 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
/// NOTE: Please do not use this directly! It is only meant to be used by the
/// optimizer pass: SILMoveOnlyWrappedTypeEliminator.
bool unsafelyEliminateMoveOnlyWrapper(const SILFunction *fn) {
if (!Type.isMoveOnlyWrapped() && !Type.isBoxedMoveOnlyWrappedType(fn))
if (!Type.hasAnyMoveOnlyWrapping(fn))
return false;
if (Type.isMoveOnlyWrapped()) {
Type = Type.removingMoveOnlyWrapper();
} else {
assert(Type.isBoxedMoveOnlyWrappedType(fn));
Type = Type.removingMoveOnlyWrapperToBoxedType(fn);
}
Type = Type.removingAnyMoveOnlyWrapping(fn);
return true;
}

Expand Down
13 changes: 12 additions & 1 deletion lib/SIL/IR/SILType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1266,7 +1266,7 @@ SILType SILType::addingMoveOnlyWrapperToBoxedType(const SILFunction *fn) {
return SILType::getPrimitiveObjectType(newBoxType);
}

SILType SILType::removingMoveOnlyWrapperToBoxedType(const SILFunction *fn) {
SILType SILType::removingMoveOnlyWrapperFromBoxedType(const SILFunction *fn) {
auto boxTy = castTo<SILBoxType>();
auto *oldLayout = boxTy->getLayout();
auto oldField = oldLayout->getFields()[0];
Expand All @@ -1284,6 +1284,17 @@ SILType SILType::removingMoveOnlyWrapperToBoxedType(const SILFunction *fn) {
return SILType::getPrimitiveObjectType(newBoxType);
}

SILType SILType::removingAnyMoveOnlyWrapping(const SILFunction *fn) {
if (!isMoveOnlyWrapped() && !isBoxedMoveOnlyWrappedType(fn))
return *this;

if (isMoveOnlyWrapped())
return removingMoveOnlyWrapper();

assert(isBoxedMoveOnlyWrappedType(fn));
return removingMoveOnlyWrapperFromBoxedType(fn);
}

bool SILType::isSendable(SILFunction *fn) const {
return getASTType()->isSendableType();
}
Expand Down
8 changes: 5 additions & 3 deletions lib/SIL/Verifier/SILVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6548,9 +6548,11 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
"Operand value should be an object");
require(cvt->getOperand()->getType().isBoxedMoveOnlyWrappedType(cvt->getFunction()),
"Operand should be move only wrapped");
require(cvt->getType() ==
cvt->getOperand()->getType().removingMoveOnlyWrapperToBoxedType(cvt->getFunction()),
"Result and operand must have the same type, today.");
require(
cvt->getType() ==
cvt->getOperand()->getType().removingMoveOnlyWrapperFromBoxedType(
cvt->getFunction()),
"Result and operand must have the same type, today.");
}

void checkCopyableToMoveOnlyWrapperValueInst(
Expand Down
86 changes: 50 additions & 36 deletions lib/SILOptimizer/Mandatory/MoveOnlyWrappedTypeEliminator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,6 @@ namespace {

struct SILMoveOnlyWrappedTypeEliminatorVisitor
: SILInstructionVisitor<SILMoveOnlyWrappedTypeEliminatorVisitor, bool> {
const llvm::SmallSetVector<SILArgument *, 8> &touchedArgs;

SILMoveOnlyWrappedTypeEliminatorVisitor(
const llvm::SmallSetVector<SILArgument *, 8> &touchedArgs)
: touchedArgs(touchedArgs) {}

bool visitSILInstruction(SILInstruction *inst) {
llvm::errs() << "Unhandled SIL Instruction: " << *inst;
llvm_unreachable("error");
Expand Down Expand Up @@ -293,56 +287,76 @@ static bool isMoveOnlyWrappedTrivial(SILValue value) {
}

bool SILMoveOnlyWrappedTypeEliminator::process() {
bool madeChange = true;
bool madeChange = false;

llvm::SmallSetVector<SILArgument *, 8> touchedArgs;
llvm::SmallSetVector<SILInstruction *, 8> touchedInsts;

// For each value whose type is move-only wrapped:
// - rewrite the value's type
// - record its users for later visitation
auto visitValue = [&touchedInsts, fn = fn,
trivialOnly = trivialOnly](SILValue value) -> bool {
if (!value->getType().hasAnyMoveOnlyWrapping(fn))
return false;

// If we are looking at trivial only, skip non-trivial function args.
if (trivialOnly && !isMoveOnlyWrappedTrivial(value))
return false;

for (auto *use : value->getNonTypeDependentUses())
touchedInsts.insert(use->getUser());

if (isa<SILUndef>(value))
value->replaceAllUsesWith(
SILUndef::get(fn, value->getType().removingAnyMoveOnlyWrapping(fn)));
else
value->unsafelyEliminateMoveOnlyWrapper(fn);

return true;
};

for (auto &bb : *fn) {
for (auto *arg : bb.getArguments()) {
if (!arg->getType().isMoveOnlyWrapped() &&
!arg->getType().isBoxedMoveOnlyWrappedType(fn))
bool relevant = visitValue(arg);
if (!relevant)
continue;

// If we are looking at trivial only, skip non-trivial function args.
if (trivialOnly &&
!arg->getType().removingMoveOnlyWrapper().isTrivial(*fn))
continue;

arg->unsafelyEliminateMoveOnlyWrapper(fn);

// If our new type is trivial, convert the arguments ownership to
// None. Otherwise, preserve the ownership kind of the argument.
if (arg->getType().isTrivial(*fn))
arg->setOwnershipKind(OwnershipKind::None);
touchedArgs.insert(arg);
for (auto *use : arg->getNonTypeDependentUses())
touchedInsts.insert(use->getUser());

madeChange = true;
}

for (auto &ii : bb) {
for (SILValue v : ii.getResults()) {
if (!v->getType().isMoveOnlyWrapped() &&
!v->getType().isBoxedMoveOnlyWrappedType(fn))
continue;

if (trivialOnly &&
!isMoveOnlyWrappedTrivial(v))
bool touched = false;
for (SILValue value : ii.getResults()) {
bool relevant = visitValue(value);
if (!relevant)
continue;

v->unsafelyEliminateMoveOnlyWrapper(fn);
touchedInsts.insert(&ii);

// Add all users as well. This ensures we visit things like
// destroy_value and end_borrow.
for (auto *use : v->getNonTypeDependentUses())
touchedInsts.insert(use->getUser());
madeChange = true;
touched = true;
}
if (!touched)
continue;
touchedInsts.insert(&ii);

madeChange = true;
}
}
// SILFunction::undefValues may grow during the loop.
SmallVector<std::pair<SILType, SILUndef *>, 4> originalUndefs(
fn->getUndefValues());
for (auto pair : originalUndefs) {
bool relevant = visitValue(pair.second);
if (!relevant)
continue;

madeChange = true;
}

SILMoveOnlyWrappedTypeEliminatorVisitor visitor(touchedArgs);
SILMoveOnlyWrappedTypeEliminatorVisitor visitor;
while (!touchedInsts.empty()) {
visitor.visit(touchedInsts.pop_back_val());
}
Expand Down
16 changes: 16 additions & 0 deletions test/SILOptimizer/moveonly_type_eliminator.sil
Original file line number Diff line number Diff line change
Expand Up @@ -562,3 +562,19 @@ bb3(%result : @owned $@moveOnly FakeOptional<Klass>):
%result2 = moveonlywrapper_to_copyable [owned] %result : $@moveOnly FakeOptional<Klass>
return %result2 : $FakeOptional<Klass>
}

// CHECK-LABEL: sil [ossa] @debug_value_undef : {{.*}} {
// CHECK: debug_value [moveable_value_debuginfo] undef : $*Klass, var, name "s"
// CHECK-LABEL: } // end sil function 'debug_value_undef'
sil [ossa] @debug_value_undef : $@convention(thin) (@owned Klass) -> () {
bb0(%x : @owned $Klass):
%addr = alloc_stack $@moveOnly Klass
%unwrapped_addr = moveonlywrapper_to_copyable_addr %addr : $*@moveOnly Klass
store %x to [init] %unwrapped_addr : $*Klass
debug_value %addr : $*@moveOnly Klass, var, name "s", argno 1, expr op_deref
destroy_addr %addr : $*@moveOnly Klass
debug_value undef : $*@moveOnly Klass, var, name "s", argno 1, expr op_deref
dealloc_stack %addr : $*@moveOnly Klass
%retval = tuple ()
return %retval : $()
}