Skip to content

Followup for #61505 #61676

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.

Sign up for GitHub

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 4 commits into from
Oct 23, 2022
Merged

Followup for #61505 #61676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8934ca7
Revert "Update ConditionForwarding for @guaranteed forwarding phi sup…
meg-gupta Oct 21, 2022
3616fc6
Revert "Add verification to ensure we don't have redundant borrow sco…
meg-gupta Oct 21, 2022
a61de10
Update ConditionForwarding tests
meg-gupta Oct 21, 2022
730b513
[SemanticARCOpts] Remove unnecessary borrow scope for @guaranteed fun…
meg-gupta Oct 21, 2022
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
11 changes: 0 additions & 11 deletions lib/SIL/Verifier/SILVerifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4881,17 +4881,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
}

void checkBranchInst(BranchInst *BI) {
for (auto arg : BI->getArgs()) {
auto *borrow = dyn_cast<BeginBorrowInst>(arg);
if (!borrow) {
continue;
}
auto op = borrow->getOperand();
if (op->getOwnershipKind() != OwnershipKind::Guaranteed) {
continue;
}
assert(!isGuaranteedForwarding(op));
}
require(BI->getArgs().size() == BI->getDestBB()->args_size(),
"branch has wrong number of arguments for dest bb");
require(std::equal(BI->getArgs().begin(), BI->getArgs().end(),
Expand Down
7 changes: 0 additions & 7 deletions lib/SILOptimizer/SemanticARC/OwnedToGuaranteedPhiOpt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,13 +251,6 @@ bool swift::semanticarc::tryConvertOwnedPhisToGuaranteedPhis(Context &ctx) {
while (!incomingValueOperandList.empty()) {
auto incomingValueOperand = incomingValueOperandList.pop_back_val();
SILValue originalValue = originalIncomingValues.pop_back_val();
if (incomingValueOperand.isGuaranteedConsuming() &&
isa<SILFunctionArgument>(originalValue) &&
originalValue->getOwnershipKind() == OwnershipKind::Guaranteed) {
auto loc = RegularLocation::getAutoGeneratedLocation();
SILBuilderWithScope builder(incomingValueOperand.getInst());
originalValue = builder.createBeginBorrow(loc, originalValue);
}
incomingValueOperand.getOperand()->set(originalValue);
}

Expand Down
11 changes: 1 addition & 10 deletions lib/SILOptimizer/SemanticARC/OwnershipLiveRange.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,16 +310,7 @@ void OwnershipLiveRange::convertJoinedLiveRangePhiToGuaranteed(
InstModCallbacks callbacks) && {

// First convert the phi value itself to be guaranteed.
SILValue phiValue = convertIntroducerToGuaranteed(introducer);

// Then insert end_borrows at each of our destroys if we are consuming. We
// have to convert the phi to guaranteed first since otherwise, the ownership
// check when we create the end_borrows will trigger.
if (auto *phi = dyn_cast<SILPhiArgument>(phiValue)) {
if (!isGuaranteedForwardingPhi(phi)) {
insertEndBorrowsAtDestroys(phiValue, deadEndBlocks, scratch);
}
}
convertIntroducerToGuaranteed(introducer);

// Then eliminate all of the destroys...
while (!destroyingUses.empty()) {
Expand Down
8 changes: 0 additions & 8 deletions lib/SILOptimizer/Transforms/ConditionForwarding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,6 @@ bool ConditionForwarding::tryOptimize(SwitchEnumInst *SEI) {
if (!EI)
return false;

if (getFunction()->hasOwnership() && EI->hasOperand()) {
auto some = EI->getOperand();
if (some->getOwnershipKind() == OwnershipKind::Guaranteed &&
isa<SILFunctionArgument>(some)) {
return false;
}
}

if (CommonBranchBlock && PredPred != CommonBranchBlock)
return false;
CommonBranchBlock = PredPred;
Expand Down
5 changes: 2 additions & 3 deletions test/SILOptimizer/conditionforwarding_nontrivial_ossa.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,8 @@ bb6:
return %r : $()
}

// Currently, this is not optimized away because enum has @guaranteed function
// argument, which will need a borrow scope before being forwarded.
// CHECK-LABEL: sil [ossa] @simple_forwarding2 :
// CHECK: switch_enum
// CHECK-NOT: switch_enum
// CHECK-LABEL: } // end sil function 'simple_forwarding2'
sil [ossa] @simple_forwarding2 : $@convention(thin) (Builtin.Int1, @guaranteed Klass) -> () {
bb0(%0 : $Builtin.Int1, %1 : @guaranteed $Klass):
Expand Down Expand Up @@ -185,6 +183,7 @@ bb6:

// CHECK-LABEL: sil [ossa] @simple_switch_enum_forwarding2 :
// CHECK: switch_enum
// CHECK-NOT: switch_enum
// CHECK-LABEL: } // end sil function 'simple_switch_enum_forwarding2'
sil [ossa] @simple_switch_enum_forwarding2 : $@convention(thin) (@guaranteed E3) -> () {
bb0(%0 : @guaranteed $E3):
Expand Down
2 changes: 0 additions & 2 deletions test/SILOptimizer/ossa_rauw_tests.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,8 +673,6 @@ bb3:
//
// CHECK-LABEL: sil [ossa] @guaranteed_copy_rauw_owned : $@convention(thin) (@guaranteed FakeOptional<Klass>) -> () {
// CHECK-NOT: copy_value
// CHECK: begin_borrow
// CHECK-NOT: copy_value
// CHECK: } // end sil function 'guaranteed_copy_rauw_owned'
sil [ossa] @guaranteed_copy_rauw_owned : $@convention(thin) (@guaranteed FakeOptional<Klass>) -> () {
bb0(%0 : @guaranteed $FakeOptional<Klass>):
Expand Down