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.

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

Do inline for the variable initialization with NRVO function call #2592

Merged
merged 5 commits into from Nov 8, 2013

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Sep 27, 2013

Support more inlining.

@WalterBright
Copy link
Member

I'm not sure what problem this is solving?

@9rnsr
Copy link
Contributor Author

9rnsr commented Oct 11, 2013

@WalterBright From the comment in scanVar function:

The problem here is that if the initializer is a
function call that returns a struct S with a cpctor:
  S s = foo();
the postblit is done by the return statement in foo()
in s2ir.c, the intermediate code generator.
But, if foo() is inlined and now the code looks like:
  S s = x;
the postblit is not there, because such assignments
are rewritten as s.cpctor(&x) by the front end.
So, the inlining won't get the postblit called.
Work around by not inlining these cases.
A proper fix would be to move all the postblit
additions to the front end.

Historically, NRVO had mostly been handled in glue layer. However recently, all of NRVO handling were properly moved to front-end layer. So, to solve the comment issue, and to increase inline-ability, I opened this PR.

@9rnsr
Copy link
Contributor Author

9rnsr commented Oct 11, 2013

I found one more fixable issue by this.
http://d.puremagic.com/issues/show_bug.cgi?id=11224

@9rnsr
Copy link
Contributor Author

9rnsr commented Nov 1, 2013

Updated to fix issue 11394, and for that, now this is based on #2702.

WalterBright added a commit that referenced this pull request Nov 8, 2013
Do inline for the variable initialization with NRVO function call
@WalterBright WalterBright merged commit 5ea29c9 into dlang:master Nov 8, 2013
@9rnsr 9rnsr deleted the fix_inline_nrvo branch November 8, 2013 10:10
@yebblies
Copy link
Member

Uhhh... test/runnable/inline.d isn't run with -inline. I'm guessing just nobody noticed before?

@9rnsr
Copy link
Contributor Author

9rnsr commented Nov 14, 2013

test/runnable/inline.d is compiled with default permute compiler switches (-inline -release -g -O). So there's no problem.

@yebblies
Copy link
Member

Ah, that makes sense. I run my tests locally without the permuted args.

@braddr
Copy link
Member

braddr commented Nov 15, 2013

This made me curious since I couldn't remember the details. The auto-tester tests pull requests with a subset of the permuted args set, but it does include -inline: ARGS="-O -inline -release". It's purely a throughput/thoroughness trade off.

@yebblies
Copy link
Member

Yeah. I would make it do one run with all of the permutable args, but -O really hurts the compile time on my crappy desktop. It doesn't help that I'm using dm make and can't use -j. Hmm, the tester seems to be down.

@CyberShadow
Copy link
Member

This pull request introduced a regression:
https://issues.dlang.org/show_bug.cgi?id=13503

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants