From 6e26b416f4452a8ba3455dea20d125cf903e4759 Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Tue, 8 Aug 2023 15:20:36 -0700 Subject: [PATCH 1/4] [SILGen] Gardening: Moved comment. --- lib/SILGen/SILGenFunction.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/SILGen/SILGenFunction.cpp b/lib/SILGen/SILGenFunction.cpp index 70d937781dd1c..d3cf1463a3b72 100644 --- a/lib/SILGen/SILGenFunction.cpp +++ b/lib/SILGen/SILGenFunction.cpp @@ -805,8 +805,6 @@ void SILGenFunction::emitCaptures(SILLocation loc, // If this is a boxed variable, we can use it directly. if (Entry.box && entryValue->getType().getASTType() == minimalLoweredType) { - // If our captured value is a box with a moveonlywrapped type inside, - // unwrap it. auto box = ManagedValue::forBorrowedObjectRValue(Entry.box); // We can guarantee our own box to the callee. if (canGuarantee) { @@ -815,6 +813,8 @@ void SILGenFunction::emitCaptures(SILLocation loc, box = box.copy(*this, loc); } + // If our captured value is a box with a moveonlywrapped type inside, + // unwrap it. if (box.getType().isBoxedMoveOnlyWrappedType(&F)) { CleanupCloner cloner(*this, box); box = cloner.clone( From c8d1ea5aaebe83805e2ff7baa4be693c5ba7be54 Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Tue, 8 Aug 2023 15:21:50 -0700 Subject: [PATCH 2/4] [SILGen] Gardening: Line wrapping. --- lib/SILGen/SILGenFunction.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/SILGen/SILGenFunction.cpp b/lib/SILGen/SILGenFunction.cpp index d3cf1463a3b72..a1bbbc66ab1d5 100644 --- a/lib/SILGen/SILGenFunction.cpp +++ b/lib/SILGen/SILGenFunction.cpp @@ -798,7 +798,8 @@ void SILGenFunction::emitCaptures(SILLocation loc, auto entryValue = getAddressValue(val, /*forceCopy=*/false); // LValues are captured as both the box owning the value and the // address of the value. - assert(entryValue->getType().isAddress() && "no address for captured var!"); + assert(entryValue->getType().isAddress() && + "no address for captured var!"); // Boxes of opaque return values stay opaque. auto minimalLoweredType = SGM.Types.getLoweredRValueType( TypeExpansionContext::minimal(), type->getCanonicalType()); @@ -838,7 +839,8 @@ void SILGenFunction::emitCaptures(SILLocation loc, // in-place. // TODO: Use immutable box for immutable captures. auto boxTy = SGM.Types.getContextBoxTypeForCapture( - vd, minimalLoweredType, FunctionDC->getGenericEnvironmentOfContext(), + vd, minimalLoweredType, + FunctionDC->getGenericEnvironmentOfContext(), /*mutable*/ true); AllocBoxInst *allocBox = B.createAllocBox(loc, boxTy); From e894a34ef8b4d34ad9bf898aec8f2a3c48a019db Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Wed, 9 Aug 2023 16:39:07 -0700 Subject: [PATCH 3/4] [SILGen] Gardening: Deleted comment. Way back in commit d0bb0274e92038df7852908d5d9ad37746096a39 Author: Joe Groff Date: Mon Nov 23 11:47:09 2015 -0800 the convention was changed such that LValues were passed only by box rather than by box and address. Delete the comment that says that both are passed. --- lib/SILGen/SILGenFunction.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/SILGen/SILGenFunction.cpp b/lib/SILGen/SILGenFunction.cpp index a1bbbc66ab1d5..46cade5499736 100644 --- a/lib/SILGen/SILGenFunction.cpp +++ b/lib/SILGen/SILGenFunction.cpp @@ -796,8 +796,6 @@ void SILGenFunction::emitCaptures(SILLocation loc, assert(!isPack); auto entryValue = getAddressValue(val, /*forceCopy=*/false); - // LValues are captured as both the box owning the value and the - // address of the value. assert(entryValue->getType().isAddress() && "no address for captured var!"); // Boxes of opaque return values stay opaque. @@ -860,8 +858,6 @@ void SILGenFunction::emitCaptures(SILLocation loc, assert(!isPack); auto entryValue = getAddressValue(val, /*forceCopy=*/false); - // LValues are captured as both the box owning the value and the - // address of the value. assert(entryValue->getType().isAddress() && "no address for captured var!"); // Boxes of opaque return values stay opaque. From 368536ce92f6850f4a314a1a65f00f6a817cd119 Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Tue, 8 Aug 2023 16:43:10 -0700 Subject: [PATCH 4/4] [SILGen] Remove subtle identity function call. Back in 33f4f57cc47f5a13a91fec7c4cb30254a53b53b3 of https://github.com/apple/swift/pull/28044 fame, non-`CaptureKind::Constant:` uses of `Entry.val` in `SILGenFunction::emitCaptures` were replaced with a use of the newly added lambda `getAddressValue` applied at `Entry.val`. The replacement of `Entry.value` with `getAddressValue(Entry.value)` in the case of `CaptureKind::Box` had no effect. Back then, the reason was that the condition SGM.Types .getTypeLowering( valueType, TypeExpansionContext::noOpaqueTypeArchetypesSubstitution( expansion.getResilienceExpansion())) .isAddressOnly() && !entryValue->getType().isAddress() under which something other than the input was returned would never hold: CaptureKind::Box is used for LValues and those never satisfy `!entryValue->getType().isAddress()`. Since that PR, the getAddressValue lambda has grown. There are two additional aspects to consider: (1) forceCopy, (2) isPack. (1) is not relevant because `false` was being passed for the `CaptureKind::Box` case. (2) can not currently happen because pack lvalues haven't been implemented. But even if they were implemented, the argument passed to the lambda would still need to be an address. The same all holds for the `CaptureKind::ImmutableBox` case which only differs from the `CaptureKind::Box` by a couple of lines. --- lib/SILGen/SILGenFunction.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/SILGen/SILGenFunction.cpp b/lib/SILGen/SILGenFunction.cpp index 46cade5499736..6d6f7524f33af 100644 --- a/lib/SILGen/SILGenFunction.cpp +++ b/lib/SILGen/SILGenFunction.cpp @@ -795,15 +795,14 @@ void SILGenFunction::emitCaptures(SILLocation loc, case CaptureKind::Box: { assert(!isPack); - auto entryValue = getAddressValue(val, /*forceCopy=*/false); - assert(entryValue->getType().isAddress() && + assert(val->getType().isAddress() && "no address for captured var!"); // Boxes of opaque return values stay opaque. auto minimalLoweredType = SGM.Types.getLoweredRValueType( TypeExpansionContext::minimal(), type->getCanonicalType()); // If this is a boxed variable, we can use it directly. if (Entry.box && - entryValue->getType().getASTType() == minimalLoweredType) { + val->getType().getASTType() == minimalLoweredType) { auto box = ManagedValue::forBorrowedObjectRValue(Entry.box); // We can guarantee our own box to the callee. if (canGuarantee) { @@ -823,7 +822,7 @@ void SILGenFunction::emitCaptures(SILLocation loc, capturedArgs.push_back(box); if (captureCanEscape) - escapesToMark.push_back(entryValue); + escapesToMark.push_back(val); } else { // Address only 'let' values are passed by box. This isn't great, in // that a variable captured by multiple closures will be boxed for each @@ -843,7 +842,7 @@ void SILGenFunction::emitCaptures(SILLocation loc, AllocBoxInst *allocBox = B.createAllocBox(loc, boxTy); ProjectBoxInst *boxAddress = B.createProjectBox(loc, allocBox, 0); - B.createCopyAddr(loc, entryValue, boxAddress, IsNotTake, + B.createCopyAddr(loc, val, boxAddress, IsNotTake, IsInitialization); if (canGuarantee) capturedArgs.push_back( @@ -857,15 +856,14 @@ void SILGenFunction::emitCaptures(SILLocation loc, case CaptureKind::ImmutableBox: { assert(!isPack); - auto entryValue = getAddressValue(val, /*forceCopy=*/false); - assert(entryValue->getType().isAddress() && + assert(val->getType().isAddress() && "no address for captured var!"); // Boxes of opaque return values stay opaque. auto minimalLoweredType = SGM.Types.getLoweredRValueType( TypeExpansionContext::minimal(), type->getCanonicalType()); // If this is a boxed variable, we can use it directly. if (Entry.box && - entryValue->getType().getASTType() == minimalLoweredType) { + val->getType().getASTType() == minimalLoweredType) { // We can guarantee our own box to the callee. if (canGuarantee) { capturedArgs.push_back( @@ -874,7 +872,7 @@ void SILGenFunction::emitCaptures(SILLocation loc, capturedArgs.push_back(emitManagedRetain(loc, Entry.box)); } if (captureCanEscape) - escapesToMark.push_back(entryValue); + escapesToMark.push_back(val); } else { // Address only 'let' values are passed by box. This isn't great, in // that a variable captured by multiple closures will be boxed for each @@ -894,7 +892,7 @@ void SILGenFunction::emitCaptures(SILLocation loc, AllocBoxInst *allocBox = B.createAllocBox(loc, boxTy); ProjectBoxInst *boxAddress = B.createProjectBox(loc, allocBox, 0); - B.createCopyAddr(loc, entryValue, boxAddress, IsNotTake, + B.createCopyAddr(loc, val, boxAddress, IsNotTake, IsInitialization); if (canGuarantee) capturedArgs.push_back(