Skip to content

Commit d514c6e

Browse files
committed
[region-isolation] Make it harder for instruction classification and looking for base values through LookThrough insts to get out of sync.
To ensure that we preserve the correct behavior here, I added classification helper functions that classify if an instruction can be look through. I used this to drive the find base value code and added in asserts in the instruction classifier to ensure that if an instruction is ever classified as LookThrough, one of the helper routines handles it.
1 parent a2566e0 commit d514c6e

File tree

1 file changed

+61
-20
lines changed

1 file changed

+61
-20
lines changed

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

Lines changed: 61 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,10 @@ struct UseDefChainVisitor
181181
/// values. We assert that all instructions that are CONSTANT_TRANSLATION
182182
/// LookThrough to make sure they stay in sync.
183183
static bool isStaticallyLookThroughInst(SILInstruction *inst) {
184+
if (auto cast = SILDynamicCastInst::getAs(inst))
185+
if (cast.isRCIdentityPreserving())
186+
return true;
187+
184188
switch (inst->getKind()) {
185189
default:
186190
return false;
@@ -220,6 +224,37 @@ static bool isStaticallyLookThroughInst(SILInstruction *inst) {
220224
}
221225
}
222226

227+
static bool isLookThroughIfResultNonSendable(SILInstruction *inst) {
228+
switch (inst->getKind()) {
229+
default:
230+
return false;
231+
case SILInstructionKind::TupleElementAddrInst:
232+
case SILInstructionKind::StructElementAddrInst:
233+
case SILInstructionKind::RawPointerToRefInst:
234+
return true;
235+
}
236+
}
237+
238+
static bool isLookThroughIfOperandNonSendable(SILInstruction *inst) {
239+
switch (inst->getKind()) {
240+
default:
241+
return false;
242+
case SILInstructionKind::RefToRawPointerInst:
243+
return true;
244+
}
245+
}
246+
247+
static bool isLookThroughIfOperandAndResultSendable(SILInstruction *inst) {
248+
switch (inst->getKind()) {
249+
default:
250+
return false;
251+
case SILInstructionKind::UncheckedTrivialBitCastInst:
252+
case SILInstructionKind::UncheckedBitwiseCastInst:
253+
case SILInstructionKind::UncheckedValueCastInst:
254+
return true;
255+
}
256+
}
257+
223258
static SILValue getUnderlyingTrackedObjectValue(SILValue value) {
224259
auto *fn = value->getFunction();
225260
SILValue result = value;
@@ -236,35 +271,26 @@ static SILValue getUnderlyingTrackedObjectValue(SILValue value) {
236271
temp = svi->getOperand(0);
237272
}
238273

239-
if (auto cast = SILDynamicCastInst::getAs(svi)) {
240-
if (cast.isRCIdentityPreserving()) {
241-
temp = svi->getOperand(0);
242-
}
243-
}
244-
245274
// If we have a cast and our operand and result are non-Sendable, treat it
246275
// as a look through.
247-
if (isa<UncheckedTrivialBitCastInst, UncheckedBitwiseCastInst,
248-
UncheckedValueCastInst>(svi)) {
276+
if (isLookThroughIfOperandAndResultSendable(svi)) {
249277
if (isNonSendableType(svi->getType(), fn) &&
250278
isNonSendableType(svi->getOperand(0)->getType(), fn)) {
251279
temp = svi->getOperand(0);
252280
}
253281
}
254-
}
255282

256-
if (auto *r = dyn_cast<RefToRawPointerInst>(temp)) {
257-
// If our operand is a non-Sendable type, look through this instruction.
258-
if (isNonSendableType(r->getOperand()->getType(), fn)) {
259-
temp = r->getOperand();
283+
if (isLookThroughIfResultNonSendable(svi)) {
284+
if (isNonSendableType(svi->getType(), fn)) {
285+
temp = svi->getOperand(0);
286+
}
260287
}
261-
}
262288

263-
if (auto *r = dyn_cast<RawPointerToRefInst>(temp)) {
264-
// If our result is a non-Sendable type, look through this
265-
// instruction. Builtin.RawPointer is always non-Sendable.
266-
if (isNonSendableType(r->getType(), fn)) {
267-
temp = r->getOperand();
289+
if (isLookThroughIfOperandNonSendable(svi)) {
290+
// If our operand is a non-Sendable type, look through this instruction.
291+
if (isNonSendableType(svi->getOperand(0)->getType(), fn)) {
292+
temp = svi->getOperand(0);
293+
}
268294
}
269295
}
270296

@@ -1829,6 +1855,11 @@ class PartitionOpTranslator {
18291855

18301856
case TranslationSemantics::LookThrough:
18311857
assert(inst->getNumOperands() == 1);
1858+
assert((isStaticallyLookThroughInst(inst) ||
1859+
isLookThroughIfResultNonSendable(inst) ||
1860+
isLookThroughIfOperandNonSendable(inst) ||
1861+
isLookThroughIfOperandAndResultSendable(inst)) &&
1862+
"Out of sync... should return true for one of these categories!");
18321863
return translateSILLookThrough(inst->getResults(), inst->getOperand(0));
18331864

18341865
case TranslationSemantics::Store:
@@ -2259,6 +2290,7 @@ CONSTANT_TRANSLATION(DeallocPackMetadataInst, Asserting)
22592290
// of the Sendable addr. That would require adding more logic though.
22602291
#define LOOKTHROUGH_IF_NONSENDABLE_RESULT_REQUIRE_OTHERWISE(INST) \
22612292
TranslationSemantics PartitionOpTranslator::visit##INST(INST *inst) { \
2293+
assert(isLookThroughIfResultNonSendable(inst) && "Out of sync?!"); \
22622294
if (isNonSendableType(inst->getType())) { \
22632295
return TranslationSemantics::LookThrough; \
22642296
} \
@@ -2294,6 +2326,7 @@ IGNORE_IF_SENDABLE_RESULT_ASSIGN_OTHERWISE(StructExtractInst)
22942326
#define CAST_WITH_MAYBE_SENDABLE_NONSENDABLE_OP_AND_RESULT(INST) \
22952327
\
22962328
TranslationSemantics PartitionOpTranslator::visit##INST(INST *cast) { \
2329+
assert(isLookThroughIfOperandAndResultSendable(cast) && "Out of sync"); \
22972330
bool isOperandNonSendable = \
22982331
isNonSendableType(cast->getOperand()->getType()); \
22992332
bool isResultNonSendable = isNonSendableType(cast->getType()); \
@@ -2324,6 +2357,7 @@ CAST_WITH_MAYBE_SENDABLE_NONSENDABLE_OP_AND_RESULT(UncheckedValueCastInst)
23242357

23252358
TranslationSemantics
23262359
PartitionOpTranslator::visitRawPointerToRefInst(RawPointerToRefInst *r) {
2360+
assert(isLookThroughIfResultNonSendable(r) && "Out of sync");
23272361
// If our result is non sendable, perform a look through.
23282362
if (isNonSendableType(r->getType()))
23292363
return TranslationSemantics::LookThrough;
@@ -2334,6 +2368,8 @@ PartitionOpTranslator::visitRawPointerToRefInst(RawPointerToRefInst *r) {
23342368

23352369
TranslationSemantics
23362370
PartitionOpTranslator::visitRefToRawPointerInst(RefToRawPointerInst *r) {
2371+
assert(isLookThroughIfOperandNonSendable(r) && "Out of sync");
2372+
23372373
// If our source ref is non sendable, perform a look through.
23382374
if (isNonSendableType(r->getOperand()->getType()))
23392375
return TranslationSemantics::LookThrough;
@@ -2345,6 +2381,7 @@ PartitionOpTranslator::visitRefToRawPointerInst(RefToRawPointerInst *r) {
23452381

23462382
TranslationSemantics
23472383
PartitionOpTranslator::visitMarkDependenceInst(MarkDependenceInst *mdi) {
2384+
assert(isStaticallyLookThroughInst(mdi) && "Out of sync");
23482385
translateSILLookThrough(mdi->getResults(), mdi->getValue());
23492386
translateSILRequire(mdi->getBase());
23502387
return TranslationSemantics::Special;
@@ -2360,8 +2397,12 @@ PartitionOpTranslator::visitPointerToAddressInst(PointerToAddressInst *ptai) {
23602397

23612398
TranslationSemantics PartitionOpTranslator::visitUnconditionalCheckedCastInst(
23622399
UnconditionalCheckedCastInst *ucci) {
2363-
if (SILDynamicCastInst(ucci).isRCIdentityPreserving())
2400+
if (SILDynamicCastInst(ucci).isRCIdentityPreserving()) {
2401+
assert(isStaticallyLookThroughInst(ucci) && "Out of sync");
23642402
return TranslationSemantics::LookThrough;
2403+
}
2404+
2405+
assert(!isStaticallyLookThroughInst(ucci) && "Out of sync");
23652406
return TranslationSemantics::Assign;
23662407
}
23672408

0 commit comments

Comments
 (0)