Skip to content

Commit c554c9b

Browse files
arifthpearifthpe
committed
Fix copy elision from tuple var
Includes maintaining a stack of nested tuple exprs used for tuple init part, alongside the type stack. Signed-off-by: Anna Rift <[email protected]>
1 parent ada9d6b commit c554c9b

File tree

3 files changed

+85
-38
lines changed

3 files changed

+85
-38
lines changed

‎frontend/lib/resolution/VarScopeVisitor.cpp‎

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,13 +204,24 @@ const AstNode* VarScopeVisitor::outermostContainingTuple() const {
204204

205205
int VarScopeVisitor::indexWithinContainingTuple(const AstNode* ast) const {
206206
CHPL_ASSERT(inAstStack.size() > 1);
207-
auto parentTuple = inAstStack[inAstStack.size() - 2]->toTupleDecl();
208-
CHPL_ASSERT(parentTuple);
207+
auto parentAst = inAstStack[inAstStack.size() - 2];
208+
209+
int parentNumDecls = -1;
209210
int indexWithinParent = -1;
210-
while (++indexWithinParent < parentTuple->numDecls() &&
211-
parentTuple->decl(indexWithinParent) != ast);
212-
CHPL_ASSERT(indexWithinParent < parentTuple->numDecls() &&
211+
if (auto tuple = parentAst->toTuple()) {
212+
parentNumDecls = tuple->numActuals();
213+
while (++indexWithinParent < parentNumDecls &&
214+
tuple->actual(indexWithinParent) != ast);
215+
} else if (auto tupleDecl = parentAst->toTupleDecl()) {
216+
parentNumDecls = tupleDecl->numDecls();
217+
while (++indexWithinParent < parentNumDecls &&
218+
tupleDecl->decl(indexWithinParent) != ast);
219+
} else {
220+
CHPL_ASSERT(false && "expected parent to be a tuple or tuple decl");
221+
}
222+
CHPL_ASSERT(indexWithinParent < parentNumDecls &&
213223
"could not find child within parent tuple decl");
224+
214225
return indexWithinParent;
215226
}
216227

@@ -330,7 +341,23 @@ bool VarScopeVisitor::enter(const TupleDecl* ast, RV& rv) {
330341
CHPL_ASSERT(initTupleType);
331342
CHPL_ASSERT(ast->numDecls() == initTupleType->numElements());
332343
}
344+
345+
const Tuple* initPart = nullptr;
346+
if (auto initExpr = ast->initExpression()) {
347+
// Get init part (if any) directly, which is possible if this is a top level
348+
// tuple decl.
349+
initPart = initExpr->toTuple();
350+
} else if (outermostContainingTuple()) {
351+
// Otherwise, see if we're nested in another tuple decl and can
352+
// derive it from our parent's.
353+
if (auto parentInit = tupleInitExprsStack.back()) {
354+
initPart = parentInit->actual(indexWithinContainingTuple(ast))->toTuple();
355+
}
356+
}
357+
if (initPart) CHPL_ASSERT(ast->numDecls() == initPart->numActuals());
358+
333359
tupleInitTypesStack.push_back(initType);
360+
tupleInitExprsStack.push_back(initPart);
334361

335362

336363
// Loop index variables don't need default-initialization and aren't
@@ -340,7 +367,18 @@ bool VarScopeVisitor::enter(const TupleDecl* ast, RV& rv) {
340367
// is there an analysis that does need to handle loop indices?
341368
// See also: skip for NamedDecl
342369
// In this tuple decl case, also prevent descending into the contained decls.
343-
return !isLoopIndex(outermostContainingTuple());
370+
if (!isLoopIndex(outermostContainingTuple())) {
371+
if (auto typeExpr = ast->typeExpression()) {
372+
typeExpr->traverse(rv);
373+
}
374+
if (auto initExpr = ast->initExpression()) {
375+
initExpr->traverse(rv);
376+
}
377+
for (auto decl : ast->decls()) {
378+
decl->traverse(rv);
379+
}
380+
}
381+
return false;
344382
}
345383
void VarScopeVisitor::exit(const TupleDecl* ast, RV& rv) {
346384
CHPL_ASSERT(!scopeStack.empty());
@@ -380,6 +418,7 @@ void VarScopeVisitor::exit(const TupleDecl* ast, RV& rv) {
380418
}
381419

382420
tupleInitTypesStack.pop_back();
421+
tupleInitExprsStack.pop_back();
383422

384423
exitScope(ast, rv);
385424
exitAst(ast);
@@ -429,6 +468,10 @@ void VarScopeVisitor::exit(const NamedDecl* ast, RV& rv) {
429468
astForDeclProps = outerTuple;
430469

431470
initExpr = outerTuple->initExpression();
471+
auto parentInitExpr = tupleInitExprsStack.back();
472+
if (parentInitExpr) {
473+
initExpr = parentInitExpr->actual(indexWithinContainingTuple(ast));
474+
}
432475
auto parentInitType = tupleInitTypesStack.back();
433476
if (!parentInitType.isUnknown()) {
434477
auto parentInitTupleType = parentInitType.type()->toTupleType();

‎frontend/lib/resolution/VarScopeVisitor.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ class VarScopeVisitor : public BranchSensitiveVisitor<VarFrame, MutatingResolved
6565
std::vector<const AstNode*> inAstStack;
6666
// Stack of tuple init/assign RHS types, matching the number and order of
6767
// tuple decls in the AST stack.
68+
// TODO: use SmallVectors
6869
std::vector<types::QualifiedType> tupleInitTypesStack;
70+
std::vector<const Tuple*> tupleInitExprsStack;
6971

7072
// ----- methods to be implemented by specific analysis subclass
7173

‎frontend/lib/resolution/copy-elision.cpp‎

Lines changed: 34 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ struct FindElidedCopies : VarScopeVisitor {
7979
static void addDeclaration(VarFrame* frame,
8080
Qualifier intentOrKind,
8181
const NamedDecl* ast);
82-
static void addCopyInit(VarFrame* frame, ID fromVarId, ID point);
82+
void addCopyInit(VarFrame* frame, ID fromVarId, ID point);
8383
static void addMention(VarFrame* frame, ID varId);
8484

8585
// save the copy-elided variables in frame to allElidedCopyFromIds
@@ -236,7 +236,9 @@ void FindElidedCopies::addCopyInit(VarFrame* frame, ID fromVarId, ID point) {
236236
// get the map entry, default-initializing it if there was none
237237
CopyElisionState& state = frame->copyElisionState[fromVarId];
238238
state.lastIsCopy = true;
239-
state.points.clear();
239+
if (!outermostContainingTuple()) {
240+
state.points.clear();
241+
}
240242
state.points.insert(point);
241243
}
242244
void FindElidedCopies::addMention(VarFrame* frame, ID varId) {
@@ -379,36 +381,36 @@ void FindElidedCopies::handleDeclaration(const VarLikeDecl* ast,
379381
if (initExpr) {
380382
VarFrame* frame = currentFrame();
381383
ID lhsVarId = ast->id();
382-
QualifiedType lhsType = rv.byId(lhsVarId).type();
383-
if (auto tupleExprInit = initExpr->toTuple()) {
384-
// handle init with tuple expression
385-
for (int i = 0; i < tupleExprInit->numActuals(); i++) {
386-
auto actual = tupleExprInit->actual(i);
387-
if (actual->isTuple()) {
388-
// processSingleDeclHelper(ast, actual, QualifiedType(),
389-
// isFormal, intentOrKind, rv);
390-
handleDeclaration(ast, parent, actual, QualifiedType(), intentOrKind,
391-
isFormal, rv);
392-
} else {
393-
ID rhsVarId = refersToId(actual, rv);
394-
if (!rhsVarId.isEmpty() && isEligibleVarInAnyFrame(rhsVarId)) {
395-
// check that the types are the same
396-
if (rv.hasId(lhsVarId) && rv.hasId(rhsVarId)) {
397-
if (lhsType.type() && lhsType.type()->isTupleType()) {
398-
const TupleType* ttype = lhsType.type()->toTupleType();
399-
CHPL_ASSERT(ttype->numElements() == tupleExprInit->numActuals());
400-
QualifiedType lhsEltType = ttype->elementType(i);
401-
402-
QualifiedType rhsType = rv.byId(rhsVarId).type();
403-
if (copyElisionAllowedForTypes(lhsEltType, rhsType, ast, rv)) {
404-
addCopyInit(frame, rhsVarId, actual->id());
405-
}
406-
}
407-
}
408-
}
409-
}
410-
}
411-
}
384+
// QualifiedType lhsType = rv.byId(lhsVarId).type();
385+
// if (auto tupleExprInit = initExpr->toTuple()) {
386+
// // handle init with tuple expression
387+
// for (int i = 0; i < tupleExprInit->numActuals(); i++) {
388+
// auto actual = tupleExprInit->actual(i);
389+
// if (actual->isTuple()) {
390+
// // processSingleDeclHelper(ast, actual, QualifiedType(),
391+
// // isFormal, intentOrKind, rv);
392+
// handleDeclaration(ast, parent, actual, QualifiedType(), intentOrKind,
393+
// isFormal, rv);
394+
// } else {
395+
// ID rhsVarId = refersToId(actual, rv);
396+
// if (!rhsVarId.isEmpty() && isEligibleVarInAnyFrame(rhsVarId)) {
397+
// // check that the types are the same
398+
// if (rv.hasId(lhsVarId) && rv.hasId(rhsVarId)) {
399+
// if (lhsType.type() && lhsType.type()->isTupleType()) {
400+
// const TupleType* ttype = lhsType.type()->toTupleType();
401+
// CHPL_ASSERT(ttype->numElements() == tupleExprInit->numActuals());
402+
// QualifiedType lhsEltType = ttype->elementType(i);
403+
404+
// QualifiedType rhsType = rv.byId(rhsVarId).type();
405+
// if (copyElisionAllowedForTypes(lhsEltType, rhsType, ast, rv)) {
406+
// addCopyInit(frame, rhsVarId, actual->id());
407+
// }
408+
// }
409+
// }
410+
// }
411+
// }
412+
// }
413+
// }
412414
ID rhsVarId = refersToId(initExpr, rv);
413415
if (!rhsVarId.isEmpty() && isEligibleVarInAnyFrame(rhsVarId)) {
414416
// check that the types are the same

0 commit comments

Comments
 (0)