Skip to content

[sil] Change all single value instructions with forwarding ownership … #20497

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 1 commit into from
Nov 12, 2018
Merged

[sil] Change all single value instructions with forwarding ownership … #20497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
65 changes: 45 additions & 20 deletions include/swift/SIL/SILBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,19 @@ class SILBuilder {
/// Reference to the provided SILBuilderContext.
SILBuilderContext &C;

/// The SILFunction that we are currently inserting into if we have one.
///
/// If we are building into a block associated with a SILGlobalVariable this
/// will be a nullptr.
///
/// TODO: This can be made cleaner by using a PointerUnion or the like so we
/// can store the SILGlobalVariable here as well.
SILFunction *F;

/// If the current block that we are inserting into must assume that
/// the current context we are in has ownership.
bool hasOwnership;
Copy link
Contributor

@atrick atrick Nov 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I hope this extra SILBuilder flag is temporary. Adding extra state in the Builder will lead to more bugs in this area.

A SILBuilder can provide its context to a new SILBuilder instance. Presumably this flag would need to be transferred to any new SILBuilders. Shouldn't it be in SILBuilderContext?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is tied to the SILFunction being passed in, so I don't think it will be an issue. That being said, I think that once I get [ossa] flag in SIL then we will be able to get rid of this.


/// If this is non-null, the instruction is inserted in the specified
/// basic block, at the specified InsertPt. If null, created instructions
/// are not auto-inserted.
Expand All @@ -133,18 +144,20 @@ class SILBuilder {

public:
explicit SILBuilder(SILFunction &F, bool isParsing = false)
: TempContext(F.getModule()), C(TempContext), F(&F), BB(0) {
: TempContext(F.getModule()), C(TempContext), F(&F),
hasOwnership(F.hasQualifiedOwnership()), BB(0) {
C.isParsing = isParsing;
}

SILBuilder(SILFunction &F, SmallVectorImpl<SILInstruction *> *InsertedInstrs)
: TempContext(F.getModule(), InsertedInstrs), C(TempContext), F(&F),
BB(0) {}
hasOwnership(F.hasQualifiedOwnership()), BB(0) {}

explicit SILBuilder(SILInstruction *I,
SmallVectorImpl<SILInstruction *> *InsertedInstrs = 0)
: TempContext(I->getFunction()->getModule(), InsertedInstrs),
C(TempContext), F(I->getFunction()) {
C(TempContext), F(I->getFunction()),
hasOwnership(F->hasQualifiedOwnership()) {
setInsertionPoint(I);
}

Expand All @@ -155,7 +168,8 @@ class SILBuilder {
explicit SILBuilder(SILBasicBlock *BB,
SmallVectorImpl<SILInstruction *> *InsertedInstrs = 0)
: TempContext(BB->getParent()->getModule(), InsertedInstrs),
C(TempContext), F(BB->getParent()) {
C(TempContext), F(BB->getParent()),
hasOwnership(F->hasQualifiedOwnership()) {
setInsertionPoint(BB);
}

Expand All @@ -165,7 +179,8 @@ class SILBuilder {
SILBuilder(SILBasicBlock *BB, SILBasicBlock::iterator InsertPt,
SmallVectorImpl<SILInstruction *> *InsertedInstrs = 0)
: TempContext(BB->getParent()->getModule(), InsertedInstrs),
C(TempContext), F(BB->getParent()) {
C(TempContext), F(BB->getParent()),
hasOwnership(F->hasQualifiedOwnership()) {
setInsertionPoint(BB, InsertPt);
}

Expand All @@ -174,7 +189,8 @@ class SILBuilder {
///
/// SILBuilderContext must outlive this SILBuilder instance.
SILBuilder(SILInstruction *I, const SILDebugScope *DS, SILBuilderContext &C)
: TempContext(C.getModule()), C(C), F(I->getFunction()) {
: TempContext(C.getModule()), C(C), F(I->getFunction()),
hasOwnership(F->hasQualifiedOwnership()) {
assert(DS && "instruction has no debug scope");
setCurrentDebugScope(DS);
setInsertionPoint(I);
Expand All @@ -185,7 +201,8 @@ class SILBuilder {
///
/// SILBuilderContext must outlive this SILBuilder instance.
SILBuilder(SILBasicBlock *BB, const SILDebugScope *DS, SILBuilderContext &C)
: TempContext(C.getModule()), C(C), F(BB->getParent()) {
: TempContext(C.getModule()), C(C), F(BB->getParent()),
hasOwnership(F->hasQualifiedOwnership()) {
assert(DS && "block has no debug scope");
setCurrentDebugScope(DS);
setInsertionPoint(BB);
Expand Down Expand Up @@ -243,6 +260,16 @@ class SILBuilder {
return SILDebugLocation(overriddenLoc, Scope);
}

/// Allow for users to override has ownership if necessary.
///
/// This is only used in the SILParser since it sets whether or not ownership
/// is qualified after the SILBuilder is constructed due to the usage of
/// AssumeUnqualifiedOwnershipWhenParsing.
///
/// TODO: Once we start printing [ossa] on SILFunctions to indicate ownership
/// and get rid of this global option, this can go away.
Copy link
Contributor

@atrick atrick Nov 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Ok, it's good that this is somehow temporary. I don't like the idea of adding flags to the SILBuilder.

void setHasOwnership(bool newHasOwnership) { hasOwnership = newHasOwnership; }

//===--------------------------------------------------------------------===//
// Insertion Point Management
//===--------------------------------------------------------------------===//
Expand Down Expand Up @@ -1187,25 +1214,23 @@ class SILBuilder {
ObjectInst *createObject(SILLocation Loc, SILType Ty,
ArrayRef<SILValue> Elements,
unsigned NumBaseElements) {
return insert(
ObjectInst::create(getSILDebugLocation(Loc), Ty, Elements,
NumBaseElements, getModule()));
return insert(ObjectInst::create(getSILDebugLocation(Loc), Ty, Elements,
NumBaseElements, getModule(),
hasOwnership));
}

StructInst *createStruct(SILLocation Loc, SILType Ty,
ArrayRef<SILValue> Elements) {
assert(Ty.isLoadableOrOpaque(getModule()));
return insert(
StructInst::create(getSILDebugLocation(Loc), Ty, Elements,
getModule()));
return insert(StructInst::create(getSILDebugLocation(Loc), Ty, Elements,
getModule(), hasOwnership));
}

TupleInst *createTuple(SILLocation Loc, SILType Ty,
ArrayRef<SILValue> Elements) {
assert(Ty.isLoadableOrOpaque(getModule()));
return insert(
TupleInst::create(getSILDebugLocation(Loc), Ty, Elements,
getModule()));
return insert(TupleInst::create(getSILDebugLocation(Loc), Ty, Elements,
getModule(), hasOwnership));
}

TupleInst *createTuple(SILLocation loc, ArrayRef<SILValue> elts);
Expand Down Expand Up @@ -1286,7 +1311,7 @@ class SILBuilder {
assert(Ty.isLoadableOrOpaque(getModule()));
return insert(SelectEnumInst::create(
getSILDebugLocation(Loc), Operand, Ty, DefaultValue, CaseValues,
getFunction(), CaseCounts, DefaultCount));
getModule(), CaseCounts, DefaultCount, hasOwnership));
}

SelectEnumAddrInst *createSelectEnumAddr(
Expand All @@ -1296,15 +1321,15 @@ class SILBuilder {
ProfileCounter DefaultCount = ProfileCounter()) {
return insert(SelectEnumAddrInst::create(
getSILDebugLocation(Loc), Operand, Ty, DefaultValue, CaseValues,
getFunction(), CaseCounts, DefaultCount));
getModule(), CaseCounts, DefaultCount));
}

SelectValueInst *createSelectValue(
SILLocation Loc, SILValue Operand, SILType Ty, SILValue DefaultResult,
ArrayRef<std::pair<SILValue, SILValue>> CaseValuesAndResults) {
return insert(SelectValueInst::create(getSILDebugLocation(Loc), Operand, Ty,
DefaultResult, CaseValuesAndResults,
getFunction()));
getModule(), hasOwnership));
}

TupleExtractInst *createTupleExtract(SILLocation Loc, SILValue Operand,
Expand Down Expand Up @@ -1491,7 +1516,7 @@ class SILBuilder {
OpenExistentialRefInst *
createOpenExistentialRef(SILLocation Loc, SILValue Operand, SILType Ty) {
auto *I = insert(new (getModule()) OpenExistentialRefInst(
getSILDebugLocation(Loc), Operand, Ty));
getSILDebugLocation(Loc), Operand, Ty, hasOwnership));
if (C.OpenedArchetypesTracker)
C.OpenedArchetypesTracker->registerOpenedArchetypes(I);
return I;
Expand Down
Loading