Skip to content
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

Fix #3778 - in-place construct <temporary>.ctor(args) #3779

Merged
merged 2 commits into from
Jul 14, 2021
Merged

Fix #3778 - in-place construct <temporary>.ctor(args) #3779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
77cebbd
Fix #3778 - in-place construct <temporary>.ctor(args)
kinke Jul 5, 2021
ecb66f6
[squash] recursive variant
kinke Jul 14, 2021
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
79 changes: 35 additions & 44 deletions gen/toir.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2744,6 +2744,18 @@ bool basetypesAreEqualWithoutModifiers(Type *l, Type *r) {
r = stripModifiers(r->toBasetype(), true);
return l->equals(r);
}

VarDeclaration *isTemporaryVar(Expression *e) {
if (auto ce = e->isCommaExp())
if (auto de = ce->getHead()->isDeclarationExp())
if (auto vd = de->declaration->isVarDeclaration())
if (vd->storage_class & STCtemp)
if (auto ve = ce->getTail()->isVarExp())
if (ve->var == vd)
return vd;

return nullptr;
}
}

bool toInPlaceConstruction(DLValue *lhs, Expression *rhs) {
Expand Down Expand Up @@ -2792,74 +2804,53 @@ bool toInPlaceConstruction(DLValue *lhs, Expression *rhs) {
return true;
}

// DMD issue 17457: detect structliteral.ctor(args)
// detect <structliteral | temporary>.ctor(args)
if (auto dve = ce->e1->isDotVarExp()) {
auto fd = dve->var->isFuncDeclaration();
if (fd && fd->isCtorDeclaration()) {
if (auto sle = dve->e1->isStructLiteralExp()) {
Logger::println("success, in-place-constructing struct literal and "
"invoking ctor");
// emit the struct literal directly into the lhs lvalue...
auto lval = DtoLVal(lhs);
ToElemVisitor::emitStructLiteral(sle, lval);
// ... and invoke the ctor directly on it
auto fnval = new DFuncValue(fd, DtoCallee(fd), lval);
Logger::println("is a constructor call, checking lhs of DotVarExp");
if (toInPlaceConstruction(lhs, dve->e1)) {
Logger::println("success, calling ctor on in-place constructed lhs");
auto fnval = new DFuncValue(fd, DtoCallee(fd), DtoLVal(lhs));
DtoCallFunction(ce->loc, ce->type, fnval, ce->arguments);
return true;
}
}
}
}

// emit struct literals directly into the lhs lvalue
if (auto sle = rhs->isStructLiteralExp()) {
else if (auto sle = rhs->isStructLiteralExp()) {
Logger::println("success, in-place-constructing struct literal");
ToElemVisitor::emitStructLiteral(sle, DtoLVal(lhs));
return true;
}

// static array literals too
Type *lhsBasetype = lhs->type->toBasetype();
if (auto al = rhs->isArrayLiteralExp()) {
if (lhsBasetype->ty == Tsarray) {
// and static array literals
else if (auto al = rhs->isArrayLiteralExp()) {
if (lhs->type->toBasetype()->ty == Tsarray) {
Logger::println("success, in-place-constructing array literal");
initializeArrayLiteral(gIR, al, DtoLVal(lhs));
return true;
}
}

// vector literals too
if (auto ve = rhs->isVectorExp()) {
// and vector literals
else if (auto ve = rhs->isVectorExp()) {
Copy link
Member

Choose a reason for hiding this comment

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

Because there is not a guaranteed return statement in the previous else-if (line 2852), this is slight change in behavior. Was that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the elses are intentional - no point in continuing the checks for other types of expressions if we know it was another one and couldn't be in-place constructed.

I might go with a recursive approach for the ctor case; I thought that was infeasible because I thought lhs was an Expression too, but it's a DValue.

Logger::println("success, in-place-constructing vector");
ToElemVisitor::emitVector(ve, DtoLVal(lhs));
return true;
}

// temporary structs and static arrays too
if (DtoIsInMemoryOnly(lhsBasetype)) {
if (auto ce = rhs->isCommaExp()) {
Expression *head = ce->getHead();
Expression *tail = ce->getTail();

if (auto de = head->isDeclarationExp()) {
if (auto vd = de->declaration->isVarDeclaration()) {
if (auto ve = tail->isVarExp()) {
if (ve->var == vd) {
Logger::println("success, in-place-constructing temporary");
auto lhsLVal = DtoLVal(lhs);
auto rhsLVal = DtoLVal(rhs);
if (!llvm::isa<llvm::AllocaInst>(rhsLVal)) {
error(rhs->loc, "lvalue of temporary is not an alloca, please "
"file an LDC issue");
fatal();
}
rhsLVal->replaceAllUsesWith(lhsLVal);
return true;
}
}
}
}
// and temporaries
else if (isTemporaryVar(rhs)) {
Logger::println("success, in-place-constructing temporary");
auto lhsLVal = DtoLVal(lhs);
auto rhsLVal = DtoLVal(rhs);
if (!llvm::isa<llvm::AllocaInst>(rhsLVal)) {
error(rhs->loc, "lvalue of temporary is not an alloca, please "
"file an LDC issue");
fatal();
}
if (lhsLVal != rhsLVal)
rhsLVal->replaceAllUsesWith(lhsLVal);
return true;
}

return false;
Expand Down
39 changes: 39 additions & 0 deletions tests/codegen/rvo.d
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// RUN: %ldc -run %s

struct S
{
int x;
S* self;
this(int x)
{
this.x = x;
self = &this;
}
~this()
{
assert(self is &this);
}
}

S makeS()
{
return S(2); // one form of RVO: <temporary>.ctor(2)
}

S nrvo()
{
S ret = makeS();
return ret;
}

S rvo()
{
return makeS();
}

void main()
{
auto s = makeS();
auto nrvo = nrvo();
auto rvo = rvo();
}