Skip to content

Commit f9b0f99

Browse files
committed
[semantic-arc] Attempt to handle indirect_mutating parameters that are never written to.
It only handles local writes, so we can not handle chains of inout parameters. That being said, I wonder if we could hook this up to an analysis to determine if the inout has this same property in callees.
1 parent 288a725 commit f9b0f99

File tree

2 files changed

+187
-4
lines changed

2 files changed

+187
-4
lines changed

Diff for: lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp

+132-4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "swift/SIL/DebugUtils.h"
1818
#include "swift/SIL/MemAccessUtils.h"
1919
#include "swift/SIL/OwnershipUtils.h"
20+
#include "swift/SIL/Projection.h"
2021
#include "swift/SIL/SILArgument.h"
2122
#include "swift/SIL/SILBuilder.h"
2223
#include "swift/SIL/SILInstruction.h"
@@ -150,6 +151,129 @@ LiveRange::LiveRange(SILValue value)
150151
}
151152
}
152153

154+
//===----------------------------------------------------------------------===//
155+
// Address Written To Analysis
156+
//===----------------------------------------------------------------------===//
157+
158+
namespace {
159+
160+
/// A simple analysis that checks if a specific def (in our case an inout
161+
/// argument) is ever written to. This is conservative, local and processes
162+
/// recursively downwards from def->use.
163+
struct IsAddressWrittenToDefUseAnalysis {
164+
llvm::SmallDenseMap<SILValue, bool, 8> isWrittenToCache;
165+
166+
bool operator()(SILValue value) {
167+
auto iter = isWrittenToCache.try_emplace(value, true);
168+
169+
// If we are already in the map, just return that.
170+
if (!iter.second)
171+
return iter.first->second;
172+
173+
// Otherwise, compute our value, cache it and return.
174+
bool result = isWrittenToHelper(value);
175+
iter.first->second = result;
176+
return result;
177+
}
178+
179+
private:
180+
bool isWrittenToHelper(SILValue value);
181+
};
182+
183+
} // end anonymous namespace
184+
185+
bool IsAddressWrittenToDefUseAnalysis::isWrittenToHelper(SILValue initialValue) {
186+
SmallVector<Operand *, 8> worklist(initialValue->getUses());
187+
while (!worklist.empty()) {
188+
auto *op = worklist.pop_back_val();
189+
SILInstruction *user = op->getUser();
190+
191+
if (Projection::isAddressProjection(user) ||
192+
isa<ProjectBlockStorageInst>(user)) {
193+
for (SILValue r : user->getResults()) {
194+
llvm::copy(r->getUses(), std::back_inserter(worklist));
195+
}
196+
continue;
197+
}
198+
199+
if (auto *oeai = dyn_cast<OpenExistentialAddrInst>(user)) {
200+
// Mutable access!
201+
if (oeai->getAccessKind() != OpenedExistentialAccess::Immutable) {
202+
return true;
203+
}
204+
205+
// Otherwise, look through it and continue.
206+
llvm::copy(oeai->getUses(), std::back_inserter(worklist));
207+
continue;
208+
}
209+
210+
// load_borrow and incidental uses are fine as well.
211+
if (isa<LoadBorrowInst>(user) || isIncidentalUse(user)) {
212+
continue;
213+
}
214+
215+
// Look through immutable begin_access.
216+
if (auto *bai = dyn_cast<BeginAccessInst>(user)) {
217+
// If we do not have a read, return true.
218+
if (bai->getAccessKind() != SILAccessKind::Read) {
219+
return true;
220+
}
221+
222+
// Otherwise, add the users to the worklist and continue.
223+
llvm::copy(bai->getUses(), std::back_inserter(worklist));
224+
continue;
225+
}
226+
227+
// As long as we do not have a load [take], we are fine.
228+
if (auto *li = dyn_cast<LoadInst>(user)) {
229+
if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Take) {
230+
return true;
231+
}
232+
continue;
233+
}
234+
235+
// If we have a FullApplySite, see if we use the value as an
236+
// indirect_guaranteed parameter. If we use it as inout, we need
237+
// interprocedural analysis that we do not perform here.
238+
if (auto fas = FullApplySite::isa(user)) {
239+
if (fas.getArgumentConvention(*op) ==
240+
SILArgumentConvention::Indirect_In_Guaranteed)
241+
continue;
242+
243+
// Otherwise, be conservative and return true.
244+
return true;
245+
}
246+
247+
// Copy addr that read are just loads.
248+
if (auto *cai = dyn_cast<CopyAddrInst>(user)) {
249+
// If our value is the destination, this is a write.
250+
if (cai->getDest() == op->get()) {
251+
return true;
252+
}
253+
254+
// Ok, so we are Src by process of elimination. Make sure we are not being
255+
// taken.
256+
if (cai->isTakeOfSrc()) {
257+
return true;
258+
}
259+
260+
// Otherwise, we are safe and can continue.
261+
continue;
262+
}
263+
264+
// If we did not recognize the user, just return conservatively that it was
265+
// written to.
266+
LLVM_DEBUG(llvm::dbgs()
267+
<< "Function: " << user->getFunction()->getName() << "\n");
268+
LLVM_DEBUG(llvm::dbgs() << "Value: " << op->get());
269+
LLVM_DEBUG(llvm::dbgs() << "Unknown instruction!: " << *user);
270+
return true;
271+
}
272+
273+
// Ok, we finished our worklist and this address is not being written to.
274+
return false;
275+
}
276+
153277
//===----------------------------------------------------------------------===//
154278
// Implementation
155279
//===----------------------------------------------------------------------===//
@@ -185,6 +309,7 @@ struct SemanticARCOptVisitor
185309
SILFunction &F;
186310
Optional<DeadEndBlocks> TheDeadEndBlocks;
187311
ValueLifetimeAnalysis::Frontier lifetimeFrontier;
312+
IsAddressWrittenToDefUseAnalysis isAddressWrittenToDefUseAnalysis;
188313

189314
explicit SemanticARCOptVisitor(SILFunction &F) : F(F) {}
190315

@@ -712,12 +837,15 @@ class StorageGuaranteesLoadVisitor
712837
return answer(false);
713838
}
714839

840+
// If we have an inout parameter that isn't ever actually written to, return
841+
// false.
842+
if (arg->getKnownParameterInfo().isIndirectMutating()) {
843+
return answer(ARCOpt.isAddressWrittenToDefUseAnalysis(arg));
844+
}
845+
715846
// TODO: This should be extended:
716847
//
717-
// 1. We should be able to analyze inout arguments and see if the inout
718-
// argument is never actually written to in a flow insensitive way.
719-
//
720-
// 2. We should be able to analyze in arguments and see if they are only
848+
// 1. We should be able to analyze in arguments and see if they are only
721849
// ever destroyed at the end of the function. In such a case, we may be
722850
// able to also to promote load [copy] from such args to load_borrow.
723851
return answer(true);

Diff for: test/SILOptimizer/semantic-arc-opts.sil

+55
Original file line numberDiff line numberDiff line change
@@ -1085,3 +1085,58 @@ bb0(%0 : @owned $Klass):
10851085
%9999 = tuple()
10861086
return %9999 : $()
10871087
}
1088+
1089+
// CHECK-LABEL: sil [ossa] @inout_argument_never_written_to_1 : $@convention(thin) (@inout NativeObjectPair) -> () {
1090+
// CHECK-NOT: load [copy]
1091+
// CHECK: load_borrow
1092+
// CHECK-NOT: load [copy]
1093+
// CHECK: } // end sil function 'inout_argument_never_written_to_1'
1094+
sil [ossa] @inout_argument_never_written_to_1 : $@convention(thin) (@inout NativeObjectPair) -> () {
1095+
bb0(%0 : $*NativeObjectPair):
1096+
%2 = load [copy] %0 : $*NativeObjectPair
1097+
(%3, %4) = destructure_struct %2 : $NativeObjectPair
1098+
1099+
%5 = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
1100+
apply %5(%3) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
1101+
apply %5(%4) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
1102+
1103+
destroy_value %3 : $Builtin.NativeObject
1104+
destroy_value %4 : $Builtin.NativeObject
1105+
1106+
%9999 = tuple()
1107+
return %9999 : $()
1108+
}
1109+
1110+
// CHECK-LABEL: sil [ossa] @inout_argument_never_written_to_2 : $@convention(thin) (@inout NativeObjectPair) -> () {
1111+
// CHECK-NOT: load [copy]
1112+
// CHECK: load_borrow
1113+
// CHECK-NOT: load [copy]
1114+
// CHECK: } // end sil function 'inout_argument_never_written_to_2'
1115+
sil [ossa] @inout_argument_never_written_to_2 : $@convention(thin) (@inout NativeObjectPair) -> () {
1116+
bb0(%0 : $*NativeObjectPair):
1117+
%2 = struct_element_addr %0 : $*NativeObjectPair, #NativeObjectPair.obj1
1118+
%3 = load [copy] %2 : $*Builtin.NativeObject
1119+
%5 = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
1120+
apply %5(%3) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
1121+
destroy_value %3 : $Builtin.NativeObject
1122+
%9999 = tuple()
1123+
return %9999 : $()
1124+
}
1125+
1126+
// We should be able to handle this, but we can not until we teach the ARC
1127+
// optimizer to avoid writes not within the load [copy] region.
1128+
//
1129+
// CHECK-LABEL: sil [ossa] @inout_argument_never_written_to_3 : $@convention(thin) (@inout NativeObjectPair, @owned Builtin.NativeObject) -> () {
1130+
// CHECK: load [copy]
1131+
// CHECK: } // end sil function 'inout_argument_never_written_to_3'
1132+
sil [ossa] @inout_argument_never_written_to_3 : $@convention(thin) (@inout NativeObjectPair, @owned Builtin.NativeObject) -> () {
1133+
bb0(%0 : $*NativeObjectPair, %1 : @owned $Builtin.NativeObject):
1134+
%2 = struct_element_addr %0 : $*NativeObjectPair, #NativeObjectPair.obj1
1135+
%3 = load [copy] %2 : $*Builtin.NativeObject
1136+
%5 = function_ref @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
1137+
apply %5(%3) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
1138+
destroy_value %3 : $Builtin.NativeObject
1139+
store %1 to [assign] %2 : $*Builtin.NativeObject
1140+
%9999 = tuple()
1141+
return %9999 : $()
1142+
}

0 commit comments

Comments
 (0)