Skip to content
This repository has been archived by the owner on Jun 20, 2019. It is now read-only.

Return the NRVO variable by reference #220

Merged
merged 1 commit into from
Jun 20, 2016

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Jun 12, 2016

This should fix the broken builds after updating to latest GCC snapshot. The test in sdtor has always been flaky because of this anyway. With this the compiler should now consistently generate the same code regardless of optimization level.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 12, 2016

Ah, I hadn't updated my local copy of gcc to the latest. I can make it work, but only with http://bugzilla.gdcproject.org/show_bug.cgi?id=42 fixed. And that breaks a lot of other stuff with it.

I'll get back in a while when I've done some triaging.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 13, 2016

And it is (more or less) working! At least the build failure is because of an error in the codegen, rather than an ICE. :-)

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 15, 2016

Split out the more complex logic in #221. Now the change is rather trivial.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 19, 2016

I might finally be at the drum-roll moment...

@ibuclaw ibuclaw force-pushed the fixnrvo branch 4 times, most recently from 0e9fa45 to 448bd06 Compare June 19, 2016 22:32
@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 20, 2016

Three failed unittests, testsuite passes. This may be a sign that there may still be more cases that cause silent corruption of codegen as a result of this.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 20, 2016

That aren't caught by testsuite nor unittester, that is.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 20, 2016

Now passing the unittester. :-o

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 20, 2016

It turned out that the frontend returns false from isPod for nested structs. However unlike structs with this(this) or ~this(), we can't pass them around by reference. But at the same time, I can't see a nice way to force them in memory.

So I'm adjusting the condition in TypeVisitor(TypeStruct) to only set TREE_ADDRESSABLE if sym>postblit || sym->dtor.

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 20, 2016

Passes!

@ibuclaw ibuclaw merged commit 517cc16 into D-Programming-GDC:master Jun 20, 2016
@ibuclaw ibuclaw deleted the fixnrvo branch June 20, 2016 09:48
@jpf91
Copy link
Contributor

jpf91 commented Jun 20, 2016

Nice work! Now I'll just have to port this to all other branches ;-)

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 20, 2016

Looks like there's more to be done. I just updated to the latest snapshot, and get more ICE's relating to setting TREE_ADDRESSABLE. This may need some ironing yet.

@jpf91
Copy link
Contributor

jpf91 commented Jun 20, 2016

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 20, 2016

Nope, I get something other. I'd imagine that some things may need to be adjusted for older versions of GCC.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants