Skip to content

Cache struct/class field offsets in SIL. #24717

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 3 commits into from
May 14, 2019
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
3 changes: 3 additions & 0 deletions include/swift/Basic/Statistics.def
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ FRONTEND_STATISTIC(AST, NumPrefixOperators)
/// Number of precedence groups in the AST context.
FRONTEND_STATISTIC(AST, NumPrecedenceGroups)

/// Number of precedence groups in the AST context.
FRONTEND_STATISTIC(AST, NumStoredPropertiesQueries)

/// Number of full function bodies parsed.
FRONTEND_STATISTIC(Parse, NumFunctionsParsed)

Expand Down
5 changes: 5 additions & 0 deletions include/swift/SIL/Projection.h
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,9 @@ class Projection {

/// Apply this projection to \p BaseType and return the relevant subfield's
/// SILType if BaseField has less subtypes than projection's offset.
///
/// WARNING: This is not a constant time operation because it is implemented
/// in terms of getVarDecl, which requests all BaseType's stored properties.
SILType getType(SILType BaseType, SILModule &M) const {
assert(isValid());
switch (getKind()) {
Expand All @@ -314,6 +317,8 @@ class Projection {
llvm_unreachable("Unhandled ProjectionKind in switch.");
}

/// WARNING: This is not a constant time operation because it requests all
/// BaseType's stored properties.
VarDecl *getVarDecl(SILType BaseType) const {
assert(isValid());
assert((getKind() == ProjectionKind::Struct ||
Expand Down
133 changes: 65 additions & 68 deletions include/swift/SIL/SILInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -5625,39 +5625,74 @@ class TupleElementAddrInst
}
};

/// Extract a physical, fragile field out of a value of struct type.
class StructExtractInst
: public UnaryInstructionBase<SILInstructionKind::StructExtractInst,
SingleValueInstruction>
{
friend SILBuilder;

VarDecl *Field;
/// A common base for instructions that require a cached field index.
///
/// "Field" is a term used here to refer to the ordered, accessible stored
/// properties of a class or struct.
///
/// The field's ordinal value is the basis of efficiently comparing and sorting
/// access paths in SIL. For example, whenever a Projection object is created,
/// it stores the field index. Finding the field index initially requires
/// searching the type declaration's array of all stored properties. If this
/// index is not cached, it will cause widespread quadratic complexity in any
/// pass that queries projections, including the SIL verifier.
///
/// FIXME: This cache may not be necessary if the Decl TypeChecker instead
/// caches a field index in the VarDecl itself. This solution would be superior
/// because it would allow constant time lookup of either the VarDecl or the
/// index from a single pointer without referring back to a projection
/// instruction.
class FieldIndexCacheBase : public SingleValueInstruction {
enum : unsigned { InvalidFieldIndex = ~unsigned(0) };

StructExtractInst(SILDebugLocation DebugLoc, SILValue Operand,
VarDecl *Field, SILType ResultTy)
: UnaryInstructionBase(DebugLoc, Operand, ResultTy), Field(Field) {}
VarDecl *field;

public:
VarDecl *getField() const { return Field; }
FieldIndexCacheBase(SILInstructionKind kind, SILDebugLocation loc,
SILType type, VarDecl *field)
: SingleValueInstruction(kind, loc, type), field(field) {
SILInstruction::Bits.FieldIndexCacheBase.FieldIndex = InvalidFieldIndex;
// This needs to be a concrete class to hold bitfield information. However,
// it should only be extended by UnaryInstructions.
assert(getNumOperands() == 1);
}

VarDecl *getField() const { return field; }

// FIXME: this should be called getFieldIndex().
unsigned getFieldNo() const {
unsigned i = 0;
for (VarDecl *D : getStructDecl()->getStoredProperties()) {
if (Field == D)
return i;
++i;
}
llvm_unreachable("A struct_extract's structdecl has at least 1 field, the "
"field of the struct_extract.");
unsigned idx = SILInstruction::Bits.FieldIndexCacheBase.FieldIndex;
if (idx != InvalidFieldIndex)
return idx;

return const_cast<FieldIndexCacheBase *>(this)->cacheFieldIndex();
}

StructDecl *getStructDecl() const {
auto s = getOperand()->getType().getStructOrBoundGenericStruct();
NominalTypeDecl *getParentDecl() const {
auto s = getOperand(0)->getType().getNominalOrBoundGenericNominal();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meta question. Should the base type of this be some sort of unary instruction base or something? Then getOperand() is correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing that out; there should be an assert. There was actually a reason I didn't make this a subclass ofUnary Instruction.
d98cf0c

assert(s);
return s;
}

private:
unsigned cacheFieldIndex();
};

/// Extract a physical, fragile field out of a value of struct type.
class StructExtractInst
: public UnaryInstructionBase<SILInstructionKind::StructExtractInst,
FieldIndexCacheBase> {
friend SILBuilder;

StructExtractInst(SILDebugLocation DebugLoc, SILValue Operand, VarDecl *Field,
SILType ResultTy)
: UnaryInstructionBase(DebugLoc, Operand, ResultTy, Field) {}

public:
StructDecl *getStructDecl() const {
return cast<StructDecl>(getParentDecl());
}

/// Returns true if this is a trivial result of a struct that is non-trivial
/// and represents one RCID.
bool isTrivialFieldOfOneRCIDStruct() const;
Expand All @@ -5670,71 +5705,33 @@ class StructExtractInst

/// Derive the address of a physical field from the address of a struct.
class StructElementAddrInst
: public UnaryInstructionBase<SILInstructionKind::StructElementAddrInst,
SingleValueInstruction>
{
: public UnaryInstructionBase<SILInstructionKind::StructElementAddrInst,
FieldIndexCacheBase> {
friend SILBuilder;

VarDecl *Field;

StructElementAddrInst(SILDebugLocation DebugLoc, SILValue Operand,
VarDecl *Field, SILType ResultTy)
: UnaryInstructionBase(DebugLoc, Operand, ResultTy), Field(Field) {}
: UnaryInstructionBase(DebugLoc, Operand, ResultTy, Field) {}

public:
VarDecl *getField() const { return Field; }

unsigned getFieldNo() const {
unsigned i = 0;
for (auto *D : getStructDecl()->getStoredProperties()) {
if (Field == D)
return i;
++i;
}
llvm_unreachable("A struct_element_addr's structdecl has at least 1 field, "
"the field of the struct_element_addr.");
}

StructDecl *getStructDecl() const {
auto s = getOperand()->getType().getStructOrBoundGenericStruct();
assert(s);
return s;
return cast<StructDecl>(getParentDecl());
}
};

/// RefElementAddrInst - Derive the address of a named element in a reference
/// type instance.
class RefElementAddrInst
: public UnaryInstructionBase<SILInstructionKind::RefElementAddrInst,
SingleValueInstruction>
{
: public UnaryInstructionBase<SILInstructionKind::RefElementAddrInst,
FieldIndexCacheBase> {
friend SILBuilder;

VarDecl *Field;

RefElementAddrInst(SILDebugLocation DebugLoc, SILValue Operand,
VarDecl *Field, SILType ResultTy)
: UnaryInstructionBase(DebugLoc, Operand, ResultTy), Field(Field) {}
: UnaryInstructionBase(DebugLoc, Operand, ResultTy, Field) {}

public:
VarDecl *getField() const { return Field; }

unsigned getFieldNo() const {
unsigned i = 0;
for (auto *D : getClassDecl()->getStoredProperties()) {
if (Field == D)
return i;
++i;
}
llvm_unreachable("A ref_element_addr's classdecl has at least 1 field, the "
"field of the ref_element_addr.");
}

ClassDecl *getClassDecl() const {
auto s = getOperand()->getType().getClassOrBoundGenericClass();
assert(s);
return s;
}
ClassDecl *getClassDecl() const { return cast<ClassDecl>(getParentDecl()); }
};

/// RefTailAddrInst - Derive the address of the first element of the first
Expand Down
4 changes: 4 additions & 0 deletions include/swift/SIL/SILNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,10 @@ class alignas(8) SILNode {
FieldNo : 32
);

SWIFT_INLINE_BITFIELD_FULL(FieldIndexCacheBase, SingleValueInstruction, 32,
: NumPadBits,
FieldIndex : 32);

SWIFT_INLINE_BITFIELD_EMPTY(MethodInst, SingleValueInstruction);
// Ensure that WitnessMethodInst bitfield does not overflow.
IBWTO_BITFIELD_EMPTY(WitnessMethodInst, MethodInst);
Expand Down
8 changes: 8 additions & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3370,6 +3370,14 @@ void NominalTypeDecl::addExtension(ExtensionDecl *extension) {

auto NominalTypeDecl::getStoredProperties(bool skipInaccessible) const
-> StoredPropertyRange {
// This should be called at most once per SIL instruction that accesses a
// VarDecl.
//
// FIXME: Once VarDecl itself caches its field index, it should be called at
// most once per finalized VarDecl.
if (getASTContext().Stats)
getASTContext().Stats->getFrontendCounters().NumStoredPropertiesQueries++;

// Clang-imported classes never have stored properties.
if (hasClangNode() && isa<ClassDecl>(this))
return StoredPropertyRange(DeclRange(nullptr, nullptr),
Expand Down
21 changes: 19 additions & 2 deletions lib/SIL/SILInstructions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,21 @@ bool TupleExtractInst::isEltOnlyNonTrivialElt() const {
return true;
}

unsigned FieldIndexCacheBase::cacheFieldIndex() {
unsigned i = 0;
for (VarDecl *property : getParentDecl()->getStoredProperties()) {
if (field == property) {
SILInstruction::Bits.FieldIndexCacheBase.FieldIndex = i;
return i;
}
++i;
}
llvm_unreachable("The field decl for a struct_extract, struct_element_addr, "
"or ref_element_addr must be an accessible stored property "
"of the operand's type");
}

// FIXME: this should be cached during cacheFieldIndex().
bool StructExtractInst::isTrivialFieldOfOneRCIDStruct() const {
auto *F = getFunction();

Expand All @@ -1011,7 +1026,7 @@ bool StructExtractInst::isTrivialFieldOfOneRCIDStruct() const {
// For each element index of the tuple...
for (VarDecl *D : getStructDecl()->getStoredProperties()) {
// If the field is the one we are extracting, skip it...
if (Field == D)
if (getField() == D)
continue;

// Otherwise check if we have a non-trivial type. If we don't have one,
Expand Down Expand Up @@ -1040,6 +1055,8 @@ bool StructExtractInst::isTrivialFieldOfOneRCIDStruct() const {
/// Return true if we are extracting the only non-trivial field of out parent
/// struct. This implies that a ref count operation on the aggregate is
/// equivalent to a ref count operation on this field.
///
/// FIXME: this should be cached during cacheFieldIndex().
bool StructExtractInst::isFieldOnlyNonTrivialField() const {
auto *F = getFunction();

Expand All @@ -1053,7 +1070,7 @@ bool StructExtractInst::isFieldOnlyNonTrivialField() const {
// Ok, we are visiting a non-trivial field. Then for every stored field...
for (VarDecl *D : getStructDecl()->getStoredProperties()) {
// If we are visiting our own field continue.
if (Field == D)
if (getField() == D)
continue;

// Ok, we have a field that is not equal to the field we are
Expand Down
6 changes: 5 additions & 1 deletion utils/scale-test
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ def run_once_with_primary(args, ast, rng, primary_idx):
else:
r = merge_all_jobstats(load_stats_dir(d)).stats
finally:
shutil.rmtree(d)
if not args.save_temps:
shutil.rmtree(d)

return {k: v for (k, v) in r.items() if args.select in k}

Expand Down Expand Up @@ -754,6 +755,9 @@ def main():
parser.add_argument(
'--tmpdir', type=str,
default=None, help='directory to create tempfiles in')
parser.add_argument(
'--save-temps', action='store_true',
default=False, help='save files in tempfiles')
parser.add_argument(
'--select',
default="", help='substring of counters/symbols to limit attention to')
Expand Down
16 changes: 16 additions & 0 deletions validation-test/compiler_scale/lazy_class_props.gyb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// RUN: %scale-test -O --begin 5 --end 21 --step 5 --select NumStoredPropertiesQueries %s
// REQUIRES: OS=macosx
// REQUIRES: asserts
//
// Single file with many stored properties.
//
// Check that SIL passes don't exhibit cubic behavior by repeatedly
// asking for all the stored properties (if the
// NumStoredPropertiesQueries stat scales quadratically, then the
// behavior is cubic).

public class LazyProperties {
%for i in range(N):
public lazy var prop${i}: String = { return "${i}" }()
%end
}