Skip to content

Commit ee17f5e

Browse files
gottesmmgottesmm
committed
[ownership] Rename OwnershipForwardingInst -> OwnershipForwardingMixin and eliminate support for isa<> and dyn_cast<>.
Instead, I just added some static helper methods that perform the same operations without needing to deal with generics/etc on OwnershipForwardingMixin itself. The reason why I did this is that this Mixin is not part of the SILNode heirarchy so we shouldn't use utilities tied to the SILNode hierarchy.
1 parent f92b75c commit ee17f5e

File tree

6 files changed

+62
-48
lines changed

6 files changed

+62
-48
lines changed

‎include/swift/SIL/SILInstruction.h

Lines changed: 49 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -929,32 +929,32 @@ inline SingleValueInstruction *SILNode::castToSingleValueInstruction() {
929929
///
930930
/// NOTE: We assume that the constructor for the instruction subclass that
931931
/// initializes the kind field on our object is run before our constructor runs.
932-
class OwnershipForwardingInst {
932+
class OwnershipForwardingMixin {
933933
ValueOwnershipKind ownershipKind;
934934

935935
protected:
936-
OwnershipForwardingInst(SILInstructionKind kind,
937-
ValueOwnershipKind ownershipKind)
936+
OwnershipForwardingMixin(SILInstructionKind kind,
937+
ValueOwnershipKind ownershipKind)
938938
: ownershipKind(ownershipKind) {
939-
assert(classof(kind) && "Invalid subclass?!");
939+
assert(isa(kind) && "Invalid subclass?!");
940940
}
941941

942942
public:
943943
ValueOwnershipKind getOwnershipKind() const { return ownershipKind; }
944944

945945
void setOwnershipKind(ValueOwnershipKind newKind) { ownershipKind = newKind; }
946946

947-
static bool classof(const SILNode *node) {
948-
if (auto *i = dyn_cast<SILInstruction>(node))
949-
return classof(i);
947+
/// Defined inline below due to forward declaration issues.
948+
static OwnershipForwardingMixin *get(SILInstruction *inst);
949+
/// Defined inline below due to forward declaration issues.
950+
static bool isa(SILInstructionKind kind);
951+
static bool isa(const SILInstruction *inst) { return isa(inst->getKind()); }
952+
static bool isa(const SILNode *node) {
953+
node = node->getRepresentativeSILNodeInObject();
954+
if (auto *i = dyn_cast<const SILInstruction>(node))
955+
return isa(i);
950956
return false;
951957
}
952-
953-
// Defined inline below due to forward declaration issues.
954-
static bool classof(const SILInstruction *inst);
955-
956-
/// Define inline below due to forward declaration issues.
957-
static bool classof(SILInstructionKind kind);
958958
};
959959

960960
/// A single value inst that forwards a static ownership from one (or all) of
@@ -964,14 +964,14 @@ class OwnershipForwardingInst {
964964
/// explicitly using setOwnershipKind().
965965
class FirstArgOwnershipForwardingSingleValueInst
966966
: public SingleValueInstruction,
967-
public OwnershipForwardingInst {
967+
public OwnershipForwardingMixin {
968968
protected:
969969
FirstArgOwnershipForwardingSingleValueInst(SILInstructionKind kind,
970970
SILDebugLocation debugLoc,
971971
SILType ty,
972972
ValueOwnershipKind ownershipKind)
973973
: SingleValueInstruction(kind, debugLoc, ty),
974-
OwnershipForwardingInst(kind, ownershipKind) {
974+
OwnershipForwardingMixin(kind, ownershipKind) {
975975
assert(classof(kind) && "classof missing new subclass?!");
976976
}
977977

@@ -1096,14 +1096,14 @@ FirstArgOwnershipForwardingSingleValueInst::classof(SILInstructionKind kind) {
10961096

10971097
class AllArgOwnershipForwardingSingleValueInst
10981098
: public SingleValueInstruction,
1099-
public OwnershipForwardingInst {
1099+
public OwnershipForwardingMixin {
11001100
protected:
11011101
AllArgOwnershipForwardingSingleValueInst(SILInstructionKind kind,
11021102
SILDebugLocation debugLoc,
11031103
SILType ty,
11041104
ValueOwnershipKind ownershipKind)
11051105
: SingleValueInstruction(kind, debugLoc, ty),
1106-
OwnershipForwardingInst(kind, ownershipKind) {
1106+
OwnershipForwardingMixin(kind, ownershipKind) {
11071107
assert(classof(kind) && "classof missing new subclass?!");
11081108
}
11091109

@@ -4680,13 +4680,13 @@ class ConversionInst : public SingleValueInstruction {
46804680
/// A conversion inst that produces a static OwnershipKind set upon the
46814681
/// instruction's construction.
46824682
class OwnershipForwardingConversionInst : public ConversionInst,
4683-
public OwnershipForwardingInst {
4683+
public OwnershipForwardingMixin {
46844684
protected:
46854685
OwnershipForwardingConversionInst(SILInstructionKind kind,
46864686
SILDebugLocation debugLoc, SILType ty,
46874687
ValueOwnershipKind ownershipKind)
46884688
: ConversionInst(kind, debugLoc, ty),
4689-
OwnershipForwardingInst(kind, ownershipKind) {
4689+
OwnershipForwardingMixin(kind, ownershipKind) {
46904690
assert(classof(kind) && "classof missing subclass?!");
46914691
}
46924692

@@ -5884,15 +5884,15 @@ class SelectEnumInstBase
58845884

58855885
/// A select enum inst that produces a static OwnershipKind.
58865886
class OwnershipForwardingSelectEnumInstBase : public SelectEnumInstBase,
5887-
public OwnershipForwardingInst {
5887+
public OwnershipForwardingMixin {
58885888
protected:
58895889
OwnershipForwardingSelectEnumInstBase(
58905890
SILInstructionKind kind, SILDebugLocation debugLoc, SILType type,
58915891
bool defaultValue, Optional<ArrayRef<ProfileCounter>> caseCounts,
58925892
ProfileCounter defaultCount, ValueOwnershipKind ownershipKind)
58935893
: SelectEnumInstBase(kind, debugLoc, type, defaultValue, caseCounts,
58945894
defaultCount),
5895-
OwnershipForwardingInst(kind, ownershipKind) {
5895+
OwnershipForwardingMixin(kind, ownershipKind) {
58965896
assert(classof(kind) && "classof missing subclass");
58975897
}
58985898

@@ -7624,12 +7624,13 @@ class TermInst : public NonValueInstruction {
76247624
};
76257625

76267626
class OwnershipForwardingTermInst : public TermInst,
7627-
public OwnershipForwardingInst {
7627+
public OwnershipForwardingMixin {
76287628
protected:
76297629
OwnershipForwardingTermInst(SILInstructionKind kind,
76307630
SILDebugLocation debugLoc,
76317631
ValueOwnershipKind ownershipKind)
7632-
: TermInst(kind, debugLoc), OwnershipForwardingInst(kind, ownershipKind) {
7632+
: TermInst(kind, debugLoc),
7633+
OwnershipForwardingMixin(kind, ownershipKind) {
76337634
assert(classof(kind));
76347635
}
76357636

@@ -9093,13 +9094,13 @@ SILFunction *ApplyInstBase<Impl, Base, false>::getCalleeFunction() const {
90939094

90949095
class OwnershipForwardingMultipleValueInstruction
90959096
: public MultipleValueInstruction,
9096-
public OwnershipForwardingInst {
9097+
public OwnershipForwardingMixin {
90979098
public:
90989099
OwnershipForwardingMultipleValueInstruction(SILInstructionKind kind,
90999100
SILDebugLocation loc,
91009101
ValueOwnershipKind ownershipKind)
91019102
: MultipleValueInstruction(kind, loc),
9102-
OwnershipForwardingInst(kind, ownershipKind) {
9103+
OwnershipForwardingMixin(kind, ownershipKind) {
91039104
assert(classof(kind) && "Missing subclass from classof?!");
91049105
}
91059106

@@ -9288,16 +9289,7 @@ inline bool Operand::isTypeDependent() const {
92889289
return getUser()->isTypeDependentOperand(*this);
92899290
}
92909291

9291-
inline bool OwnershipForwardingInst::classof(const SILInstruction *inst) {
9292-
return FirstArgOwnershipForwardingSingleValueInst::classof(inst) ||
9293-
AllArgOwnershipForwardingSingleValueInst::classof(inst) ||
9294-
OwnershipForwardingTermInst::classof(inst) ||
9295-
OwnershipForwardingConversionInst::classof(inst) ||
9296-
OwnershipForwardingSelectEnumInstBase::classof(inst) ||
9297-
OwnershipForwardingMultipleValueInstruction::classof(inst);
9298-
}
9299-
9300-
inline bool OwnershipForwardingInst::classof(SILInstructionKind kind) {
9292+
inline bool OwnershipForwardingMixin::isa(SILInstructionKind kind) {
93019293
return FirstArgOwnershipForwardingSingleValueInst::classof(kind) ||
93029294
AllArgOwnershipForwardingSingleValueInst::classof(kind) ||
93039295
OwnershipForwardingTermInst::classof(kind) ||
@@ -9306,6 +9298,28 @@ inline bool OwnershipForwardingInst::classof(SILInstructionKind kind) {
93069298
OwnershipForwardingMultipleValueInstruction::classof(kind);
93079299
}
93089300

9301+
inline OwnershipForwardingMixin *
9302+
OwnershipForwardingMixin::get(SILInstruction *inst) {
9303+
// I am purposely performing this cast in this manner rather than reinterpret
9304+
// casting to OwnershipForwardingMixin to ensure that we offset to the
9305+
// appropriate offset inside of inst instead of converting inst's current
9306+
// location to an OwnershipForwardingMixin which would be incorrect.
9307+
if (auto *result = dyn_cast<FirstArgOwnershipForwardingSingleValueInst>(inst))
9308+
return result;
9309+
if (auto *result = dyn_cast<AllArgOwnershipForwardingSingleValueInst>(inst))
9310+
return result;
9311+
if (auto *result = dyn_cast<OwnershipForwardingTermInst>(inst))
9312+
return result;
9313+
if (auto *result = dyn_cast<OwnershipForwardingConversionInst>(inst))
9314+
return result;
9315+
if (auto *result = dyn_cast<OwnershipForwardingSelectEnumInstBase>(inst))
9316+
return result;
9317+
if (auto *result =
9318+
dyn_cast<OwnershipForwardingMultipleValueInstruction>(inst))
9319+
return result;
9320+
return nullptr;
9321+
}
9322+
93099323
} // end swift namespace
93109324

93119325
//===----------------------------------------------------------------------===//

‎lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,15 @@ bool swift::canOpcodeForwardGuaranteedValues(SILValue value) {
7979
if (auto *arg = dyn_cast<SILArgument>(value))
8080
if (auto *ti = arg->getSingleTerminator())
8181
if (ti->isTransformationTerminator()) {
82-
assert(isa<OwnershipForwardingInst>(ti));
82+
assert(OwnershipForwardingMixin::isa(ti));
8383
return true;
8484
}
8585

8686
auto *node = value->getRepresentativeSILNodeInObject();
8787
bool result = isGuaranteedForwardingValueKind(node->getKind());
8888
if (result) {
8989
assert(!isa<OwnedFirstArgForwardingSingleValueInst>(node));
90-
assert(isa<OwnershipForwardingInst>(node));
90+
assert(OwnershipForwardingMixin::isa(node));
9191
}
9292
return result;
9393
}
@@ -98,7 +98,7 @@ bool swift::canOpcodeForwardGuaranteedValues(Operand *use) {
9898
bool result = isOwnershipForwardingValueKind(SILNodeKind(kind));
9999
if (result) {
100100
assert(!isa<GuaranteedFirstArgForwardingSingleValueInst>(user));
101-
assert(isa<OwnershipForwardingInst>(user));
101+
assert(OwnershipForwardingMixin::isa(user));
102102
}
103103
return result;
104104
}
@@ -118,14 +118,14 @@ bool swift::canOpcodeForwardOwnedValues(SILValue value) {
118118
if (auto *arg = dyn_cast<SILPhiArgument>(value))
119119
if (auto *predTerm = arg->getSingleTerminator())
120120
if (predTerm->isTransformationTerminator()) {
121-
assert(isa<OwnershipForwardingInst>(predTerm));
121+
assert(OwnershipForwardingMixin::isa(predTerm));
122122
return true;
123123
}
124124
auto *node = value->getRepresentativeSILNodeInObject();
125125
bool result = isOwnedForwardingValueKind(node->getKind());
126126
if (result) {
127127
assert(!isa<GuaranteedFirstArgForwardingSingleValueInst>(node));
128-
assert(isa<OwnershipForwardingInst>(node));
128+
assert(OwnershipForwardingMixin::isa(node));
129129
}
130130
return result;
131131
}
@@ -135,7 +135,7 @@ bool swift::canOpcodeForwardOwnedValues(Operand *use) {
135135
auto kind = SILNodeKind(user->getKind());
136136
bool result = isOwnershipForwardingValueKind(kind);
137137
if (result) {
138-
assert(isa<OwnershipForwardingInst>(user));
138+
assert(OwnershipForwardingMixin::isa(user));
139139
}
140140
return result;
141141
}
@@ -873,7 +873,7 @@ Optional<ForwardingOperand> ForwardingOperand::get(Operand *use) {
873873
if (use->isTypeDependent())
874874
return None;
875875

876-
if (!isa<OwnershipForwardingInst>(use->getUser())) {
876+
if (!OwnershipForwardingMixin::isa(use->getUser())) {
877877
return None;
878878
}
879879
#ifndef NDEBUG

‎lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1136,7 +1136,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
11361136
}
11371137
}
11381138

1139-
if (isa<OwnershipForwardingInst>(I)) {
1139+
if (OwnershipForwardingMixin::isa(I)) {
11401140
checkOwnershipForwardingInst(I);
11411141
}
11421142
}

‎lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,7 @@ bool SemanticARCOptVisitor::tryPerformOwnedCopyValueOptimization(
763763
SmallVector<Operand *, 8> parentLifetimeEndingUses;
764764
for (auto *origValueUse : originalValue->getUses())
765765
if (origValueUse->isLifetimeEnding() &&
766-
!isa<OwnershipForwardingInst>(origValueUse->getUser()))
766+
!OwnershipForwardingMixin::isa(origValueUse->getUser()))
767767
parentLifetimeEndingUses.push_back(origValueUse);
768768

769769
// Ok, we have an owned value. If we do not have any non-destroying consuming

‎lib/SILOptimizer/SemanticARC/SemanticARCOptVisitor.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ struct LLVM_LIBRARY_VISIBILITY SemanticARCOptVisitor
122122

123123
bool visitSILInstruction(SILInstruction *i) {
124124
assert((isa<OwnershipForwardingTermInst>(i) ||
125-
!isa<OwnershipForwardingInst>(i)) &&
125+
!OwnershipForwardingMixin::isa(i)) &&
126126
"Should have forwarding visitor for all ownership forwarding "
127127
"non-term instructions");
128128
return false;
@@ -132,7 +132,7 @@ struct LLVM_LIBRARY_VISIBILITY SemanticARCOptVisitor
132132
bool visitValueBase(ValueBase *v) {
133133
auto *inst = v->getDefiningInstruction();
134134
(void)inst;
135-
assert((!inst || !isa<OwnershipForwardingInst>(inst)) &&
135+
assert((!inst || !OwnershipForwardingMixin::isa(inst)) &&
136136
"Should have forwarding visitor for all ownership forwarding "
137137
"instructions");
138138
return false;

‎lib/SILOptimizer/Utils/CanonicalizeInstruction.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ eliminateUnneededForwardingUnarySingleValueInst(SingleValueInstruction *inst,
539539
static Optional<SILBasicBlock::iterator>
540540
tryEliminateUnneededForwardingInst(SILInstruction *i,
541541
CanonicalizeInstruction &pass) {
542-
assert(isa<OwnershipForwardingInst>(i) &&
542+
assert(OwnershipForwardingMixin::isa(i) &&
543543
"Must be an ownership forwarding inst");
544544
if (auto *svi = dyn_cast<SingleValueInstruction>(i))
545545
if (svi->getNumOperands() == 1)
@@ -576,7 +576,7 @@ CanonicalizeInstruction::canonicalize(SILInstruction *inst) {
576576
// diagnostics.
577577
auto *fn = inst->getFunction();
578578
if (fn->hasOwnership() && fn->getModule().getStage() != SILStage::Raw) {
579-
if (isa<OwnershipForwardingInst>(inst))
579+
if (OwnershipForwardingMixin::isa(inst))
580580
if (auto newNext = tryEliminateUnneededForwardingInst(inst, *this))
581581
return *newNext;
582582
}

0 commit comments

Comments
 (0)