[dmd-cxx] Move check for NRVO from ReturnStatement semantic into FuncDeclaration

[ibuclaw]

Backport of #8467. This is a companion patch for NRVO related fixes in pr d/96156 and d/96157.

[dlang-bot]

Thanks for your pull request, @ibuclaw!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "dmd-cxx + dmd#11622"

[Geod24]

Nice! There are still a few checks in the backend though, do you plan to move those too ?

[ibuclaw]

Nice! There are still a few checks in the backend though, do you plan to move those too ?

There are a few common ABI optimizations at play here.

1. Fully addressable/non-trivial types: All copies of an object are enforced by front-end language semantics (back-end/optimizer cannot do anything that causes a copy operation). These are always passed and returned by reference.
2. Call slot optimization (or RVO): A type that must return in memory by ABI of the target (e.g: because it doesn't fit in registers) becomes the first parameter of a function, which is used as the return value.
3. Direct construction at return address: Returning a literal that has a constructor usually creates a temporary for the 'this' pointer. If the type is returning in memory anyway, code generator can elide the temporary and pass the return address to the constructor instead.

NRVO is just applied on top of 1 or 2 by binding a variable name to the result decl.

Of the above, [1] is already managed by the front-end, though changing types to be ref is handled by the code generator. [2] is target specific, and an optimization, so out of scope of what the language should guarantee. [3] The front-end does not have the notion of a result declaration (return expr; is really return <result> = expr), nor does the front-end create such temporaries for literals whose address needs to be taken (AST looks like return S().__ctor(1, 2, 3)) when we really generate return <result> = S(), __ctor(&<result> 1, 2, 3)).

[ibuclaw]

Which is a long winded way of say: While there could be some improvements in hinting what the code generator should be generating, rather than second-guessing the AST. So long as this feature is only an optimization, there's no desire to put new rules to enforce behaviour in the front-end.

[Geod24]

The underlying reason for my question is: https://issues.dlang.org/show_bug.cgi?id=20752
At the moment using non-copy-able types is still a pain (and those types aren't really non-copy-able).

[ibuclaw]

isReturnOnStack is correct (hence no static assert), but it does not mean that all copied would be elided along the way. That's what @disable is meant for.

[Geod24]

but it does not mean that all copied would be elided along the way

AFAIR, that's not what is happening here. I think it might be:

dmd/src/dmd/toir.d

Lines 737 to 743 in c15a596

	/* Can't do nrvo if the variable is put in a closure, since
	* what the shidden points to may no longer exist.
	*/
	if (fd.nrvo_can && fd.nrvo_var == v)
	{
	fd.nrvo_can = false;
	}

That's what @disable is meant for.

There are (documented) cases where DMD flat out ignore @disable and does the move anyway.

[ibuclaw]

AFAIR, that's not what is happening here. I think it might be:

Maybe, but both gdc and ldc should be mimicing that behaviour anyway. (On another note, I don't think there's any specific test that is designed to hit that code path).

There are (documented) cases where DMD flat out ignore @disable and does the move anyway.

Would be interesting to see them tried with gdc. If there's any violations then an ICE would occur. If no ICE occurs and you still get a bad program, then the problem is in the front-end, not the code generator.