This is an archive of the discontinued LLVM Phabricator instance.

Generalize NRVO to cover C structs
ClosedPublic

Authored by ahatanak on Mar 27 2018, 8:37 PM.

Details

Summary

r326307 and r327870 made changes that allowed using non-trivial C
structs with fields qualified with strong or weak. This commit generalizes NRVO, which could only be applied to C++ structs, to cover non-trivial C structs.

Diff Detail

Repository
rL LLVM

Event Timeline

ahatanak created this revision.Mar 27 2018, 8:37 PM

Is it possible to just do this for all structs? I don't think it hurts anything to do it for structs that are trivial and returned indirectly, and it would certainly be nice to construct C return values in-place even if they're guilty of nothing more than being, y'know, really really big.

lib/CodeGen/CGDecl.cpp
1116 ↗(On Diff #140034)

I think this cast isn't necessary.

ahatanak updated this revision to Diff 140119.Mar 28 2018, 12:21 PM
ahatanak marked an inline comment as done.
ahatanak retitled this revision from [ObjC] Generalize NRVO to cover non-trivial C structs to [ObjC] Generalize NRVO to cover C structs.

Yes, it's possible to do this for all C structs. It removes lots of alloca and memcpy instructions in the IR.

Wow, the IR improvements here are amazing.

lib/CodeGen/CGDecl.cpp
1119 ↗(On Diff #140119)

This should be isNonTrivialToPrimitiveDestroy(), I think. The dynamic NRVO flag is specifically tied to whether we should destroy the local variable in its normal cleanup.

ahatanak updated this revision to Diff 140146.Mar 28 2018, 3:28 PM
ahatanak marked an inline comment as done.
ahatanak retitled this revision from [ObjC] Generalize NRVO to cover C structs to Generalize NRVO to cover C structs.

Use isNonTrivialToPrimitiveDestroy instead of isNonTrivialToPrimitiveCopy.

rjmccall accepted this revision.Mar 28 2018, 3:32 PM

LGTM!

This revision is now accepted and ready to land.Mar 28 2018, 3:32 PM
This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.