diff --git a/compiler/src/dmd/expressionsem.d b/compiler/src/dmd/expressionsem.d index f5aeb6131803..810cf3b78cc8 100644 --- a/compiler/src/dmd/expressionsem.d +++ b/compiler/src/dmd/expressionsem.d @@ -761,6 +761,10 @@ bool isLvalue(Expression _this) * Determine if copy elision is allowed when copying an expression to * a typed storage. This basically elides a restricted subset of so-called * "pure" rvalues, i.e. expressions with no reference semantics. + * + * Note: Please try to keep `dmd.glue.e2ir.toElem()` in sync with this. + * It is not destructive to fail to elide a copy, but it is always better + * to stay consistent. */ bool canElideCopy(Expression e, Type to, bool checkMod = true) { diff --git a/compiler/src/dmd/glue/e2ir.d b/compiler/src/dmd/glue/e2ir.d index e7f9815207ac..1826183090ee 100644 --- a/compiler/src/dmd/glue/e2ir.d +++ b/compiler/src/dmd/glue/e2ir.d @@ -704,7 +704,7 @@ elem* toElem(Expression e, ref IRState irs, elem* ehidden = null) } } - Symbol* s = toSymbol(se.var); + Symbol* s = toSymbolNRVO(se.var); // VarExp generated for `__traits(initSymbol, Aggregate)`? if (auto symDec = se.var.isSymbolDeclaration()) @@ -731,9 +731,7 @@ elem* toElem(Expression e, ref IRState irs, elem* ehidden = null) if (se.var.toParent2()) fd = se.var.toParent2().isFuncDeclaration(); - const bool nrvo = fd && (fd.isNRVO && fd.nrvo_var == se.var || se.var.nrvo && fd.shidden); - if (nrvo) - s = cast(Symbol*)fd.shidden; + const bool nrvo = fd && s == fd.shidden; if (s.Sclass == SC.auto_ || s.Sclass == SC.parameter || s.Sclass == SC.shadowreg) { @@ -2631,46 +2629,6 @@ elem* toElem(Expression e, ref IRState irs, elem* ehidden = null) return setResult2(e); } - /* This will work if we can distinguish an assignment from - * an initialization of the lvalue. It'll work if the latter. - * If the former, because of aliasing of the return value with - * function arguments, it'll fail. - */ - if (ae.op == EXP.construct) - { - if (CallExp ce = lastComma(ae.e2).isCallExp()) - { - TypeFunction tf = cast(TypeFunction)ce.e1.type.toBasetype(); - if (tf.ty == Tfunction && retStyle(tf, ce.f && ce.f.needThis()) == RET.stack) - { - elem* eh = el_una(OPaddr, TYnptr, e1); - elem* e = toElem(ae.e2, irs, eh); - return setResult2(e); - } - - /* Look for: - * v = structliteral.ctor(args) - * and have the structliteral write into v, rather than create a temporary - * and copy the temporary into v - */ - if (e1.Eoper == OPvar && // no closure variables https://issues.dlang.org/show_bug.cgi?id=17622 - ae.e1.op == EXP.variable && ce.e1.op == EXP.dotVariable) - { - auto dve = cast(DotVarExp)ce.e1; - auto fd = dve.var.isFuncDeclaration(); - if (fd && fd.isCtorDeclaration()) - { - if (auto sle = dve.e1.isStructLiteralExp()) - { - sle.sym = toSymbol((cast(VarExp)ae.e1).var); - elem* e = toElem(ae.e2, irs); - return setResult2(e); - } - } - } - } - } - //if (ae.op == EXP.construct) printf("construct\n"); if (auto t1s = t1b.isTypeStruct()) { @@ -2739,6 +2697,15 @@ elem* toElem(Expression e, ref IRState irs, elem* ehidden = null) } } + /* Implement: + * S struct = func() + */ + if (ae.op == EXP.construct && canElideCopy(ae.e2, ae.e1.type, false)) + { + elem* e = toElem(ae.e2, irs, e1); + return setResult2(e); + } + /* Implement: * (struct = struct) */ @@ -2827,6 +2794,15 @@ elem* toElem(Expression e, ref IRState irs, elem* ehidden = null) } } + /* Implement: + * S[n] sarray = func() + */ + if (ae.op == EXP.construct && canElideCopy(ae.e2, ae.e1.type, false)) + { + elem* e = toElem(ae.e2, irs, e1); + return setResult2(e); + } + /* https://issues.dlang.org/show_bug.cgi?id=13661 * Even if the elements in rhs are all rvalues and * don't have to call postblits, this assignment should call @@ -3202,15 +3178,17 @@ elem* toElem(Expression e, ref IRState irs, elem* ehidden = null) auto ctfecond = ce.econd.op == EXP.not ? (cast(NotExp)ce.econd).e1 : ce.econd; if (auto ve = ctfecond.isVarExp()) if (ve.var && ve.var.ident == Id.ctfe) - return toElem(ctfecond is ce.econd ? ce.e2 : ce.e1, irs); + return toElem(ctfecond is ce.econd ? ce.e2 : ce.e1, irs, ehidden); elem* ec = toElem(ce.econd, irs); - elem* eleft = toElem(ce.e1, irs); + elem* eleft = toElem(ce.e1, irs, ehidden); if (irs.params.cov && ce.e1.loc.linnum) eleft = el_combine(incUsageElem(irs, ce.e1.loc), eleft); - elem* eright = toElem(ce.e2, irs); + if (ehidden) + ehidden = el_copytree(ehidden); + elem* eright = toElem(ce.e2, irs, ehidden); if (irs.params.cov && ce.e2.loc.linnum) eright = el_combine(incUsageElem(irs, ce.e2.loc), eright); @@ -3486,7 +3464,62 @@ elem* toElem(Expression e, ref IRState irs, elem* ehidden = null) sle.useStaticInit = false; // don't modify initializer, so make copy } - ec = toElem(dve.e1, irs); + // Special cases for constructor RVO + if (ehidden && fd && fd.isCtorDeclaration()) + { + auto le = lastComma(dve.e1); + auto sle = le.isStructLiteralExp(); + auto ve = le.isVarExp(); + + if (sle && ehidden.Eoper == OPvar) + { + // Initialization in these forms: + // S s1 = S( ... ).__ctor( ... ); + // return S( ... ).__ctor( ... ); + sle.sym = ehidden.Vsym; + ec = toElem(dve.e1, irs); + } + else if (sle || ve && (ve.var.storage_class | STC.temp) != 0) + { + // Initialization in the form: + // this.field = S( ... ).__ctor( ... ); + // Or constructor calls on a temporary: + // S s1 = ( ... , tmp).__ctor( ... ); + // return ( ... , tmp).__ctor( ... ); + // Generate: + // (ehidden=(..., tmp), ehidden).__ctor() + elem* eh = el_copytree(ehidden); + elem* el = toElem(dve.e1, irs); + + if (tybasic(eh.Ety) == TYnptr) + eh = el_una(OPind, el.Ety, eh); + + elem* ea = elAssign(eh, el, dve.e1.type, null); + ec = el_combine(ea, ehidden); + + if (tybasic(ec.Ety) != TYnptr) + ec = el_una(OPaddr, TYnptr, ec); + } + else + { + // User-written explicit constructor call: + // S s2 = s1.__ctor( ... ); + // Generate code without RVO, then blit to ehidden. + elem* eh = el_copytree(ehidden); + elem* el = toElem(ce, irs); + + if (tybasic(eh.Ety) == TYnptr) + eh = el_una(OPind, el.Ety, eh); + + elem* ea = elAssign(eh, el, ce.type, null); + return el_combine(ea, ehidden); + } + + ehidden = null; + } + else + ec = toElem(dve.e1, irs); + ectype = dve.e1.type.toBasetype(); /* Recognize: @@ -3625,6 +3658,12 @@ elem* toElem(Expression e, ref IRState irs, elem* ehidden = null) } } } + + if (ehidden && tybasic(ehidden.Ety) != TYnptr) + { + ehidden = el_una(OPaddr, TYnptr, ehidden); + } + elem* ethis2 = null; if (ce.vthis2) { @@ -4243,6 +4282,80 @@ elem* toElem(Expression e, ref IRState irs, elem* ehidden = null) return el_ptr(toSymbol(e)); } + /*****************************************************/ + /* RVO special cases */ + /*****************************************************/ + + elem* moveToHiddenPtr(Expression e) + { + // Generate (ehidden=e, ehidden) + elem* eh = el_copytree(ehidden); + elem* el = toElem(e, irs); + + if (tybasic(eh.Ety) == TYnptr) + eh = el_una(OPind, el.Ety, eh); + + elem* ea = elAssign(eh, el, e.type, null); + return el_combine(ea, ehidden); + } + + elem* doCallRVO(CallExp e) + { + FuncDeclaration fd; + + if (auto dve = e.e1.isDotVarExp()) + fd = dve.var.isFuncDeclaration(); + else + fd = e.f; + + // If fd is eligible for RVO, invoke visitCall directly to pick up ehidden + Type t = e.e1.type.toBasetype(); + if (t.ty == Tdelegate) + t = t.nextOf(); + if (fd && fd.isCtorDeclaration() || + t.ty == Tfunction && retStyle(cast(TypeFunction)t, fd && fd.needThis()) == RET.stack) + return visitCall(e); + + return moveToHiddenPtr(e); + } + + elem* doVariableRVO(VarExp e) + { + // Replace e with ehidden if e.var is already the hidden variable + if (ehidden.Eoper == OPvar && ehidden.Vsym == toSymbolNRVO(e.var)) + return ehidden; + + return moveToHiddenPtr(e); + } + + elem* doStructLiteralRVO(StructLiteralExp e) + { + // Fix up e.sym if there is a symbol to emplace into + if (ehidden.Eoper == OPvar) + { + e.sym = ehidden.Vsym; + return toElem(e, irs); + } + + return moveToHiddenPtr(e); + } + + if (ehidden) + { + // Catch unhandled RVO cases + // Please keep in sync with dmd.expressionsem.canElideCopy() + switch (e.op) + { + case EXP.comma: return visitComma(e.isCommaExp()); + case EXP.question: return visitCond(e.isCondExp()); + case EXP.call: return doCallRVO(e.isCallExp()); + case EXP.variable: return doVariableRVO(e.isVarExp()); + case EXP.dotVariable: return moveToHiddenPtr(e); + case EXP.structLiteral: return doStructLiteralRVO(e.isStructLiteralExp()); + default: assert(0); + } + } + switch (e.op) { default: return visit(e); diff --git a/compiler/src/dmd/glue/s2ir.d b/compiler/src/dmd/glue/s2ir.d index e39a21fc5a51..c6322a1ea191 100644 --- a/compiler/src/dmd/glue/s2ir.d +++ b/compiler/src/dmd/glue/s2ir.d @@ -38,7 +38,7 @@ import dmd.dsymbol; import dmd.dstruct; import dmd.dtemplate; import dmd.expression; -import dmd.expressionsem : getDsymbol, toInteger; +import dmd.expressionsem : canElideCopy, getDsymbol, toInteger; import dmd.func; import dmd.id; import dmd.init; @@ -553,143 +553,95 @@ void Statement_toIR(Statement s, ref IRState irs, StmtState* stmtstate) void visitReturn(ReturnStatement s) { //printf("s2ir.ReturnStatement: %s\n", toChars(s.exp)); - BlockState* blx = irs.blx; - BC bc; - incUsage(irs, s.loc); - void finish() + void finish(elem* e = null) { - block* finallyBlock; - if (config.ehmethod != EHmethod.EH_DWARF && - !irs.isNothrow() && - (finallyBlock = stmtstate.getFinallyBlock()) != null) + BC bc = BC.ret; + BlockState* blx = irs.blx; + + if (e) { - assert(finallyBlock.bc == BC._finally); - blx.curblock.appendSucc(finallyBlock); + elem_setLoc(e, s.loc); + block_appendexp(blx.curblock, e); + bc = BC.retexp; + } + + if (config.ehmethod != EHmethod.EH_DWARF &&!irs.isNothrow()) + { + if (block* finallyBlock = stmtstate.getFinallyBlock()) + { + assert(finallyBlock.bc == BC._finally); + blx.curblock.appendSucc(finallyBlock); + } } block_setLoc(blx.curblock, s.loc); block_next(blx, bc, null); } + + incUsage(irs, s.loc); + if (!s.exp) - { - bc = BC.ret; return finish(); - } - - elem* e; FuncDeclaration func = irs.getFunc(); assert(func); auto tf = func.type.isTypeFunction(); assert(tf); - RET retmethod = retStyle(tf, func.needThis()); - if (retmethod == RET.stack) + if (retStyle(tf, func.needThis()) == RET.stack) { - elem* es; - bool writetohp; + bool nrvo = func.isNRVO && func.nrvo_var; + bool urvo = !nrvo && canElideCopy(s.exp, s.exp.type, false); - /* If returning struct literal, write result - * directly into return value - */ - if (auto sle = s.exp.isStructLiteralExp()) - { - sle.sym = irs.shidden; - writetohp = true; - } - /* Detect function call that returns the same struct - * and construct directly into *shidden - */ - else if (auto ce = s.exp.isCallExp()) - { - if (ce.e1.op == EXP.variable || ce.e1.op == EXP.star) - { - Type t = ce.e1.type.toBasetype(); - if (t.ty == Tdelegate) - t = t.nextOf(); - if (t.ty == Tfunction && retStyle(cast(TypeFunction)t, ce.f && ce.f.needThis()) == RET.stack) - { - e = toElemDtor(s.exp, irs, el_var(irs.shidden)); - e = el_una(OPaddr, TYnptr, e); - goto L1; - } - } - else if (auto dve = ce.e1.isDotVarExp()) - { - auto fd = dve.var.isFuncDeclaration(); - if (fd && fd.isCtorDeclaration()) - { - if (auto sle = dve.e1.isStructLiteralExp()) - { - sle.sym = irs.shidden; - writetohp = true; - } - } - Type t = ce.e1.type.toBasetype(); - if (t.ty == Tdelegate) - t = t.nextOf(); - if (t.ty == Tfunction && retStyle(cast(TypeFunction)t, fd && fd.needThis()) == RET.stack) - { - e = toElemDtor(s.exp, irs, el_var(irs.shidden)); - e = el_una(OPaddr, TYnptr, e); - goto L1; - } - } - } - e = toElemDtor(s.exp, irs); + // Pass shidden in ehidden for URVO + elem* ehidden = urvo ? el_var(irs.shidden) : null; + elem* e = toElemDtor(s.exp, irs, ehidden); assert(e); - if (writetohp || - (func.isNRVO && func.nrvo_var)) + if (urvo || nrvo) { - // Return value via hidden pointer passed as parameter - // Write exp; return shidden; - es = e; + // In RVO cases, toElemDtor already ensures e references + // the hidden pointer, so there is no need to rewrite + e = el_una(OPaddr, TYnptr, e); } else { // Return value via hidden pointer passed as parameter - // Write *shidden=exp; return shidden; - es = el_una(OPind,e.Ety,el_var(irs.shidden)); + // Rewrite to (*shidden=exp, shidden) + elem* es = el_una(OPind, e.Ety, el_var(irs.shidden)); es = elAssign(es, e, s.exp.type, null); + e = el_combine(es, el_var(irs.shidden)); } - e = el_var(irs.shidden); - e = el_bin(OPcomma, e.Ety, es, e); + + return finish(e); } - else if (tf.isRef) - { - // Reference return, so convert to a pointer - e = toElemDtor(s.exp, irs); + elem* e = toElemDtor(s.exp, irs); + assert(e); + + if (tf.isRef) + { /* already taken care of for vresult in buildResultVar() and semantic3.d * https://issues.dlang.org/show_bug.cgi?id=19384 */ if (func.vresult) + { if (BlitExp be = s.exp.isBlitExp()) { if (VarExp ve = be.e1.isVarExp()) { if (ve.var == func.vresult) - goto Lskip; + return finish(e); } } + } + // Reference return, so convert to a pointer e = addressElem(e, s.exp.type.pointerTo()); - Lskip: } - else - { - e = toElemDtor(s.exp, irs); - assert(e); - } - L1: - elem_setLoc(e, s.loc); - block_appendexp(blx.curblock, e); - bc = BC.retexp; -// if (type_zeroCopy(Type_toCtype(s.exp.type))) -// bc = BC.ret; - finish(); + + finish(e); } /************************************** diff --git a/compiler/src/dmd/glue/tocsym.d b/compiler/src/dmd/glue/tocsym.d index 9bc127b5066c..db9538379198 100644 --- a/compiler/src/dmd/glue/tocsym.d +++ b/compiler/src/dmd/glue/tocsym.d @@ -104,6 +104,7 @@ Symbol* toSymbolX(Dsymbol ds, const(char)* prefix, SC sclass, type* t, const(cha } /************************************* + * Convert D symbol to backend symbol. */ Symbol* toSymbol(Dsymbol s) @@ -551,6 +552,27 @@ Symbol* toSymbol(Dsymbol s) return v.result; } +/************************************* + * Convert D symbol to backend symbol. + * Unlike toSymbol, this function also resolves + * NRVO variables to the hidden pointer. + */ +Symbol* toSymbolNRVO(Dsymbol s) +{ + if (auto parent = s.toParent2()) + { + auto fd = parent.isFuncDeclaration(); + auto var = s.isVarDeclaration(); + bool nrvo = fd && var && (fd.isNRVO && fd.nrvo_var == var || var.nrvo && fd.shidden); + + if (nrvo) + { + return cast(Symbol*)fd.shidden; + } + } + + return toSymbol(s); +} /************************************* * Create Windows import symbol from backend Symbol. diff --git a/compiler/test/runnable/nrvo.d b/compiler/test/runnable/nrvo.d index bceba100e0bd..71a11768b738 100644 --- a/compiler/test/runnable/nrvo.d +++ b/compiler/test/runnable/nrvo.d @@ -49,8 +49,127 @@ void test2() /***************************************************/ +struct S3 +{ + S3* ptr; + + this(int) + { + ptr = &this; + } + + @disable this(this); + + void check() + { + assert(this.ptr is &this); + } + + invariant(this.ptr !is null); +} + +S3 make3() +{ + return S3(1); +} + +__gshared int i3; +__gshared bool b3; + +S3 f3() +{ + i3++; + return make3(); +} + +S3 g3() +{ + return b3 ? make3() : f3(); +} + +void test3() +{ + S3 s1 = f3(); + s1.check(); + assert(i3 == 1); + + S3 s2 = b3 ? f3() : g3(); + s2.check(); + assert(i3 == 2); + + S3 s3 = f3(); + auto closure = { s3.check(); }; + closure(); + s3.check(); + assert(i3 == 3); +} + +/***************************************************/ + +// ditto + +struct S4 +{ + S4* ptr; + + this(int) + { + ptr = &this; + } + + this(ref inout S4) + { + ptr = &this; + } + + this(S4) + { + ptr = &this; + } + + void check() + { + assert(this.ptr is &this); + } + + invariant(this.ptr !is null); +} + +struct V4 +{ + S4 s; + this(int) + { + s = S4(1); + } +} + +S4 f4() +{ + return __rvalue(V4(1).s); +} + +S4 g4() +{ + V4 v = V4(1); + S4 s = v.s; + return s; +} + +void test4() +{ + f4().check(); + + S4 s = g4(); + s.check(); +} + +/***************************************************/ + void main() { test1(); test2(); + test3(); + test4(); } diff --git a/compiler/test/runnable/rvalue1.d b/compiler/test/runnable/rvalue1.d index e50ccd083957..cdc2a1d40584 100644 --- a/compiler/test/runnable/rvalue1.d +++ b/compiler/test/runnable/rvalue1.d @@ -275,32 +275,6 @@ void test11() /********************************/ -struct S12{ - S12* ptr; - this(int) { ptr = &this; } - this(ref inout S12) { ptr = &this; } - this(S12) { ptr = &this; } -} - -struct V12 -{ - S12 s; - this(int) { s = S12(1); } -} - -S12 foo12() -{ - return __rvalue(V12(1).s); -} - -void test12() -{ - S12 s = foo12(); - assert(&s == s.ptr); -} - -/********************************/ - int main() { test1(); @@ -314,7 +288,6 @@ int main() test9(); test10(); test11(); - test12(); return 0; } diff --git a/compiler/test/runnable/test28.d b/compiler/test/runnable/test28.d index 06c77467c2e9..d521ef68c1b1 100644 --- a/compiler/test/runnable/test28.d +++ b/compiler/test/runnable/test28.d @@ -1272,42 +1272,6 @@ void test18576() assert(&s == callback.ptr); } -/*******************************************/ - -struct S67 -{ - S67* ptr; - - this(int) - { - pragma(inline, false); - ptr = &this; - } - - @disable this(this); -} - -pragma(inline, false) -S67 make67() -{ - return S67(1); -} - -__gshared int i67; - -S67 f67() -out (s; s.ptr == &s) -{ - i67++; - return make67(); -} - -void test67() -{ - S67 s = f67(); - assert(s.ptr == &s); -} - /*******************************************/ // https://github.com/dlang/dmd/issues/22160 @@ -1458,7 +1422,6 @@ void main() test64(); test65(); test18576(); - test67(); test22160(); test22292();