Skip to content

Commit 0a5ba3f

Browse files
committed
Refine AccessUseDefChainVisitor.
Prepare to reuse this visitor for an AccessPath utility. Remove visitIncomplete. Add visitCast and visitPathComponent. Handle phis in a separate visitor. This simplifies the main visitor. In the long-term, we may be able to eliminate the pointer-phi visitor entirely. For now, this lets us enforce that all phi paths follow the same access path.
1 parent 8af488e commit 0a5ba3f

File tree

3 files changed

+317
-225
lines changed

3 files changed

+317
-225
lines changed

include/swift/SIL/MemAccessUtils.h

Lines changed: 167 additions & 175 deletions
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ void checkSwitchEnumBlockArg(SILPhiArgument *arg);
631631
/// Return true if the given address producer may be the source of a formal
632632
/// access (a read or write of a potentially aliased, user visible variable).
633633
///
634-
/// `storage` must be a valid AccessedStorage object.
634+
/// `storage` must be a valid, non-nested AccessedStorage object.
635635
///
636636
/// If this returns false, then the address can be safely accessed without
637637
/// a begin_access marker. To determine whether to emit begin_access:
@@ -670,12 +670,6 @@ class AccessUseDefChainVisitor {
670670
Impl &asImpl() { return static_cast<Impl &>(*this); }
671671

672672
public:
673-
// Subclasses can provide a method for any identified access base:
674-
// Result visitBase(SILValue base, AccessedStorage::Kind kind);
675-
676-
// Visitors for specific identified access kinds. These default to calling out
677-
// to visitIdentified.
678-
679673
Result visitClassAccess(RefElementAddrInst *field) {
680674
return asImpl().visitBase(field, AccessedStorage::Class);
681675
}
@@ -685,7 +679,7 @@ class AccessUseDefChainVisitor {
685679
Result visitBoxAccess(AllocBoxInst *box) {
686680
return asImpl().visitBase(box, AccessedStorage::Box);
687681
}
688-
/// The argument may be either a GlobalAddrInst or the ApplyInst for a global
682+
/// \p global may be either a GlobalAddrInst or the ApplyInst for a global
689683
/// accessor function.
690684
Result visitGlobalAccess(SILValue global) {
691685
return asImpl().visitBase(global, AccessedStorage::Global);
@@ -699,192 +693,190 @@ class AccessUseDefChainVisitor {
699693
Result visitNestedAccess(BeginAccessInst *access) {
700694
return asImpl().visitBase(access, AccessedStorage::Nested);
701695
}
702-
703-
// Visitors for unidentified base values.
704-
705696
Result visitUnidentified(SILValue base) {
706697
return asImpl().visitBase(base, AccessedStorage::Unidentified);
707698
}
708699

709-
// Subclasses must provide implementations to visit non-access bases
710-
// and phi arguments, and for incomplete projections from the access:
711-
// void visitNonAccess(SILValue base);
712-
// void visitPhi(SILPhiArgument *phi);
713-
// void visitIncomplete(SILValue projectedAddr, SILValue parentAddr);
700+
// Subclasses must provide implementations for:
701+
//
702+
// Result visitBase(SILValue base, AccessedStorage::Kind kind);
703+
// Result visitNonAccess(SILValue base);
704+
// Result visitPhi(SILPhiArgument *phi);
705+
// Result visitCast(SingleValueInstruction *cast, Operand *parentAddr);
706+
// Result visitPathComponent(SingleValueInstruction *projectedAddr,
707+
// Operand *parentAddr);
714708

715709
Result visit(SILValue sourceAddr);
716710
};
717711

718712
template<typename Impl, typename Result>
719713
Result AccessUseDefChainVisitor<Impl, Result>::visit(SILValue sourceAddr) {
720-
// Handle immediately-identifiable instructions.
721-
while (true) {
722-
switch (sourceAddr->getKind()) {
723-
// An AllocBox is a fully identified memory location.
724-
case ValueKind::AllocBoxInst:
725-
return asImpl().visitBoxAccess(cast<AllocBoxInst>(sourceAddr));
726-
// An AllocStack is a fully identified memory location, which may occur
727-
// after inlining code already subjected to stack promotion.
728-
case ValueKind::AllocStackInst:
729-
return asImpl().visitStackAccess(cast<AllocStackInst>(sourceAddr));
730-
case ValueKind::GlobalAddrInst:
731-
return asImpl().visitGlobalAccess(sourceAddr);
732-
case ValueKind::ApplyInst: {
733-
FullApplySite apply(cast<ApplyInst>(sourceAddr));
734-
if (auto *funcRef = apply.getReferencedFunctionOrNull()) {
735-
if (getVariableOfGlobalInit(funcRef)) {
736-
return asImpl().visitGlobalAccess(sourceAddr);
737-
}
714+
switch (sourceAddr->getKind()) {
715+
default:
716+
if (isAddressForLocalInitOnly(sourceAddr))
717+
return asImpl().visitUnidentified(sourceAddr);
718+
return asImpl().visitNonAccess(sourceAddr);
719+
720+
// MARK: Handle immediately-identifiable instructions.
721+
722+
// An AllocBox is a fully identified memory location.
723+
case ValueKind::AllocBoxInst:
724+
return asImpl().visitBoxAccess(cast<AllocBoxInst>(sourceAddr));
725+
// An AllocStack is a fully identified memory location, which may occur
726+
// after inlining code already subjected to stack promotion.
727+
case ValueKind::AllocStackInst:
728+
return asImpl().visitStackAccess(cast<AllocStackInst>(sourceAddr));
729+
case ValueKind::GlobalAddrInst:
730+
return asImpl().visitGlobalAccess(sourceAddr);
731+
case ValueKind::ApplyInst: {
732+
FullApplySite apply(cast<ApplyInst>(sourceAddr));
733+
if (auto *funcRef = apply.getReferencedFunctionOrNull()) {
734+
if (getVariableOfGlobalInit(funcRef)) {
735+
return asImpl().visitGlobalAccess(sourceAddr);
738736
}
739-
// Try to classify further below.
740-
break;
741-
}
742-
case ValueKind::RefElementAddrInst:
743-
return asImpl().visitClassAccess(cast<RefElementAddrInst>(sourceAddr));
744-
// A yield is effectively a nested access, enforced independently in
745-
// the caller and callee.
746-
case ValueKind::BeginApplyResult:
747-
return asImpl().visitYieldAccess(cast<BeginApplyResult>(sourceAddr));
748-
// A function argument is effectively a nested access, enforced
749-
// independently in the caller and callee.
750-
case ValueKind::SILFunctionArgument:
751-
return asImpl().visitArgumentAccess(cast<SILFunctionArgument>(sourceAddr));
752-
// View the outer begin_access as a separate location because nested
753-
// accesses do not conflict with each other.
754-
case ValueKind::BeginAccessInst:
755-
return asImpl().visitNestedAccess(cast<BeginAccessInst>(sourceAddr));
756-
default:
757-
// Try to classify further below.
758-
break;
759737
}
760-
761-
// If the sourceAddr producer cannot immediately be classified, follow the
762-
// use-def chain of sourceAddr, box, or RawPointer producers.
763-
assert(sourceAddr->getType().isAddress()
764-
|| isa<SILBoxType>(sourceAddr->getType().getASTType())
765-
|| isa<BuiltinRawPointerType>(sourceAddr->getType().getASTType()));
766-
767-
// Handle other unidentified address sources.
768-
switch (sourceAddr->getKind()) {
769-
default:
770-
if (isAddressForLocalInitOnly(sourceAddr))
771-
return asImpl().visitUnidentified(sourceAddr);
772-
return asImpl().visitNonAccess(sourceAddr);
773-
774-
case ValueKind::SILUndef:
738+
if (isExternalGlobalAddressor(cast<ApplyInst>(sourceAddr)))
775739
return asImpl().visitUnidentified(sourceAddr);
776740

777-
case ValueKind::ApplyInst:
778-
if (isExternalGlobalAddressor(cast<ApplyInst>(sourceAddr)))
779-
return asImpl().visitUnidentified(sourceAddr);
780-
781-
// Don't currently allow any other calls to return an accessed address.
782-
return asImpl().visitNonAccess(sourceAddr);
783-
784-
case ValueKind::StructExtractInst:
785-
// Handle nested access to a KeyPath projection. The projection itself
786-
// uses a Builtin. However, the returned UnsafeMutablePointer may be
787-
// converted to an address and accessed via an inout argument.
788-
if (isUnsafePointerExtraction(cast<StructExtractInst>(sourceAddr)))
789-
return asImpl().visitUnidentified(sourceAddr);
790-
return asImpl().visitNonAccess(sourceAddr);
791-
792-
case ValueKind::SILPhiArgument: {
793-
auto *phiArg = cast<SILPhiArgument>(sourceAddr);
794-
if (phiArg->isPhiArgument()) {
795-
return asImpl().visitPhi(phiArg);
796-
}
797-
798-
// A non-phi block argument may be a box value projected out of
799-
// switch_enum. Address-type block arguments are not allowed.
800-
if (sourceAddr->getType().isAddress())
801-
return asImpl().visitNonAccess(sourceAddr);
802-
803-
checkSwitchEnumBlockArg(cast<SILPhiArgument>(sourceAddr));
741+
// Don't currently allow any other calls to return an accessed address.
742+
return asImpl().visitNonAccess(sourceAddr);
743+
}
744+
case ValueKind::RefElementAddrInst:
745+
return asImpl().visitClassAccess(cast<RefElementAddrInst>(sourceAddr));
746+
// A yield is effectively a nested access, enforced independently in
747+
// the caller and callee.
748+
case ValueKind::BeginApplyResult:
749+
return asImpl().visitYieldAccess(cast<BeginApplyResult>(sourceAddr));
750+
// A function argument is effectively a nested access, enforced
751+
// independently in the caller and callee.
752+
case ValueKind::SILFunctionArgument:
753+
return asImpl().visitArgumentAccess(cast<SILFunctionArgument>(sourceAddr));
754+
755+
// View the outer begin_access as a separate location because nested
756+
// accesses do not conflict with each other.
757+
case ValueKind::BeginAccessInst:
758+
return asImpl().visitNestedAccess(cast<BeginAccessInst>(sourceAddr));
759+
760+
case ValueKind::SILUndef:
761+
return asImpl().visitUnidentified(sourceAddr);
762+
763+
// MARK: The sourceAddr producer cannot immediately be classified,
764+
// follow the use-def chain.
765+
766+
case ValueKind::StructExtractInst:
767+
// Handle nested access to a KeyPath projection. The projection itself
768+
// uses a Builtin. However, the returned UnsafeMutablePointer may be
769+
// converted to an address and accessed via an inout argument.
770+
if (isUnsafePointerExtraction(cast<StructExtractInst>(sourceAddr)))
804771
return asImpl().visitUnidentified(sourceAddr);
772+
return asImpl().visitNonAccess(sourceAddr);
773+
774+
case ValueKind::SILPhiArgument: {
775+
auto *phiArg = cast<SILPhiArgument>(sourceAddr);
776+
if (phiArg->isPhiArgument()) {
777+
return asImpl().visitPhi(phiArg);
805778
}
806-
// Load a box from an indirect payload of an opaque enum.
807-
// We must have peeked past the project_box earlier in this loop.
808-
// (the indirectness makes it a box, the load is for address-only).
809-
//
810-
// %payload_adr = unchecked_take_enum_data_addr %enum : $*Enum, #Enum.case
811-
// %box = load [take] %payload_adr : $*{ var Enum }
812-
//
813-
// FIXME: this case should go away with opaque values.
814-
//
815-
// Otherwise return invalid AccessedStorage.
816-
case ValueKind::LoadInst:
817-
if (sourceAddr->getType().is<SILBoxType>()) {
818-
SILValue operAddr = cast<LoadInst>(sourceAddr)->getOperand();
819-
assert(isa<UncheckedTakeEnumDataAddrInst>(operAddr));
820-
return asImpl().visitIncomplete(sourceAddr, operAddr);
821-
}
822-
return asImpl().visitNonAccess(sourceAddr);
823779

824-
// ref_tail_addr project an address from a reference.
825-
// This is a valid address producer for nested @inout argument
826-
// access, but it is never used for formal access of identified objects.
827-
case ValueKind::RefTailAddrInst:
828-
return asImpl().visitUnidentified(sourceAddr);
780+
// A non-phi block argument may be a box value projected out of
781+
// switch_enum. Address-type block arguments are not allowed.
782+
if (sourceAddr->getType().isAddress())
783+
return asImpl().visitNonAccess(sourceAddr);
829784

830-
// Inductive single-operand cases:
831-
// Look through address casts to find the source address.
832-
case ValueKind::MarkUninitializedInst:
833-
case ValueKind::OpenExistentialAddrInst:
834-
case ValueKind::UncheckedAddrCastInst:
835-
// Inductive cases that apply to any type.
836-
case ValueKind::CopyValueInst:
837-
case ValueKind::MarkDependenceInst:
838-
// Look through a project_box to identify the underlying alloc_box as the
839-
// accesed object. It must be possible to reach either the alloc_box or the
840-
// containing enum in this loop, only looking through simple value
841-
// propagation such as copy_value.
842-
case ValueKind::ProjectBoxInst:
843-
// Handle project_block_storage just like project_box.
844-
case ValueKind::ProjectBlockStorageInst:
845-
// Look through begin_borrow in case a local box is borrowed.
846-
case ValueKind::BeginBorrowInst:
847-
return asImpl().visitIncomplete(sourceAddr,
848-
cast<SingleValueInstruction>(sourceAddr)->getOperand(0));
849-
850-
// Access to a Builtin.RawPointer. Treat this like the inductive cases above
851-
// because some RawPointers originate from identified locations. See the
852-
// special case for global addressors, which return RawPointer,
853-
// above. AddressToPointer is also handled because it results from inlining
854-
// a global addressor without folding the
855-
// AddressToPointer->PointerToAddress.
856-
//
857-
// If the inductive search does not find a valid addressor, it will
858-
// eventually reach the default case that returns in invalid location. This
859-
// is correct for RawPointer because, although accessing a RawPointer is
860-
// legal SIL, there is no way to guarantee that it doesn't access class or
861-
// global storage, so returning a valid unidentified storage object would be
862-
// incorrect. It is the caller's responsibility to know that formal access
863-
// to such a location can be safely ignored.
864-
//
865-
// For example:
866-
//
867-
// - KeyPath Builtins access RawPointer. However, the caller can check
868-
// that the access `isFromBuilin` and ignore the storage.
869-
//
870-
// - lldb generates RawPointer access for debugger variables, but SILGen
871-
// marks debug VarDecl access as 'Unsafe' and SIL passes don't need the
872-
// AccessedStorage for 'Unsafe' access.
873-
case ValueKind::PointerToAddressInst:
874-
case ValueKind::AddressToPointerInst:
875-
return asImpl().visitIncomplete(sourceAddr,
876-
cast<SingleValueInstruction>(sourceAddr)->getOperand(0));
877-
878-
// Address-to-address subobject projections.
879-
case ValueKind::StructElementAddrInst:
880-
case ValueKind::TupleElementAddrInst:
881-
case ValueKind::UncheckedTakeEnumDataAddrInst:
882-
case ValueKind::TailAddrInst:
883-
case ValueKind::IndexAddrInst:
884-
return asImpl().visitIncomplete(sourceAddr,
885-
cast<SingleValueInstruction>(sourceAddr)->getOperand(0));
785+
checkSwitchEnumBlockArg(cast<SILPhiArgument>(sourceAddr));
786+
return asImpl().visitUnidentified(sourceAddr);
787+
}
788+
// Load a box from an indirect payload of an opaque enum.
789+
// We must have peeked past the project_box earlier in this loop.
790+
// (the indirectness makes it a box, the load is for address-only).
791+
//
792+
// %payload_adr = unchecked_take_enum_data_addr %enum : $*Enum, #Enum.case
793+
// %box = load [take] %payload_adr : $*{ var Enum }
794+
//
795+
// FIXME: this case should go away with opaque values.
796+
//
797+
// Otherwise return invalid AccessedStorage.
798+
case ValueKind::LoadInst:
799+
if (sourceAddr->getType().is<SILBoxType>()) {
800+
Operand *addrOper = &cast<LoadInst>(sourceAddr)->getOperandRef();
801+
assert(isa<UncheckedTakeEnumDataAddrInst>(addrOper->get()));
802+
return asImpl().visitCast(cast<SingleValueInstruction>(sourceAddr),
803+
addrOper);
886804
}
887-
};
805+
return asImpl().visitNonAccess(sourceAddr);
806+
807+
// ref_tail_addr project an address from a reference.
808+
// This is a valid address producer for nested @inout argument
809+
// access, but it is never used for formal access of identified objects.
810+
case ValueKind::RefTailAddrInst:
811+
return asImpl().visitUnidentified(sourceAddr);
812+
813+
// Inductive single-operand cases:
814+
// Look through address casts to find the source address.
815+
case ValueKind::MarkUninitializedInst:
816+
case ValueKind::OpenExistentialAddrInst:
817+
case ValueKind::UncheckedAddrCastInst:
818+
// Inductive cases that apply to any type.
819+
case ValueKind::CopyValueInst:
820+
case ValueKind::MarkDependenceInst:
821+
// Look through a project_box to identify the underlying alloc_box as the
822+
// accesed object. It must be possible to reach either the alloc_box or the
823+
// containing enum in this loop, only looking through simple value
824+
// propagation such as copy_value.
825+
case ValueKind::ProjectBoxInst:
826+
// Handle project_block_storage just like project_box.
827+
case ValueKind::ProjectBlockStorageInst:
828+
// Look through begin_borrow in case a local box is borrowed.
829+
case ValueKind::BeginBorrowInst:
830+
// Casting to RawPointer does not affect the AccessPath. When converting
831+
// between address types, they must be layout compatible (with truncation).
832+
case ValueKind::AddressToPointerInst:
833+
// A tail_addr is a projection that does not affect the access path because it
834+
// must always originate from a ref_tail_addr. Any projection within the
835+
// object's tail storage effectively has the same access path.
836+
case ValueKind::TailAddrInst:
837+
return asImpl().visitCast(
838+
cast<SingleValueInstruction>(sourceAddr),
839+
&cast<SingleValueInstruction>(sourceAddr)->getAllOperands()[0]);
840+
841+
// Access to a Builtin.RawPointer. It may be important to continue looking
842+
// through this because some RawPointers originate from identified
843+
// locations. See the special case for global addressors, which return
844+
// RawPointer, above.
845+
//
846+
// If the inductive search does not find a valid addressor, it will
847+
// eventually reach the default case that returns in invalid location. This
848+
// is correct for RawPointer because, although accessing a RawPointer is
849+
// legal SIL, there is no way to guarantee that it doesn't access class or
850+
// global storage, so returning a valid unidentified storage object would be
851+
// incorrect. It is the caller's responsibility to know that formal access
852+
// to such a location can be safely ignored.
853+
//
854+
// For example:
855+
//
856+
// - KeyPath Builtins access RawPointer. However, the caller can check
857+
// that the access `isFromBuilin` and ignore the storage.
858+
//
859+
// - lldb generates RawPointer access for debugger variables, but SILGen
860+
// marks debug VarDecl access as 'Unsafe' and SIL passes don't need the
861+
// AccessedStorage for 'Unsafe' access.
862+
//
863+
// This is always considered a path component because an IndexAddr may
864+
// project from it.
865+
case ValueKind::PointerToAddressInst:
866+
return asImpl().visitPathComponent(
867+
cast<SingleValueInstruction>(sourceAddr),
868+
&cast<SingleValueInstruction>(sourceAddr)->getAllOperands()[0]);
869+
870+
// Address-to-address subobject projections. Projection::isAddressProjection
871+
// returns true for these.
872+
case ValueKind::StructElementAddrInst:
873+
case ValueKind::TupleElementAddrInst:
874+
case ValueKind::UncheckedTakeEnumDataAddrInst:
875+
case ValueKind::IndexAddrInst:
876+
return asImpl().visitPathComponent(
877+
cast<SingleValueInstruction>(sourceAddr),
878+
&cast<SingleValueInstruction>(sourceAddr)->getAllOperands()[0]);
879+
}
888880
}
889881

890882
} // end namespace swift

0 commit comments

Comments
 (0)