This is an archive of the discontinued LLVM Phabricator instance.

Update NRVO logic to support early return (Attempt 2)
ClosedPublic

Authored by tzik on May 31 2018, 6:26 AM.

Details

Summary

This is the second attempt of r333500 (Update NRVO logic to support early return).
The previous one was reverted for a miscompilation for an incorrect NRVO set up on templates such as:

struct Foo {};

template <typename T>
T bar() {
  T t;
  if (false)
    return T();
  return t;
}

Where, t is marked as non-NRVO variable before its instantiation. However, while its instantiation, it's left an NRVO candidate, turned into an NRVO variable later.

Diff Detail

Repository
rC Clang

Event Timeline

tzik created this revision.May 31 2018, 6:26 AM
tzik planned changes to this revision.May 31 2018, 6:26 AM
tzik added a reviewer: rsmith.May 31 2018, 9:30 PM

rsmith: PTAL. This is the previous attempt plus a fix and a test.
The diff is SemaTemplateInstantiateDecl.cpp part that propagate disabled NRVO state to the instantiated local variable.
On the previous attempt, I left the local variable NRVO candidate on the template instnantiation, and following computeNRVO for the instantiated function template unexpectedly turned on NRVO for the candidate.

tzik added a comment.Jun 11 2018, 10:54 AM

rsmith: ping. Any chance you could review this?

rsmith accepted this revision.Jun 18 2018, 12:35 PM

LGTM

test/CodeGenCXX/nrvo-noopt.cpp
2

You don't need the -O0 here; that's the default.

This revision is now accepted and ready to land.Jun 18 2018, 12:35 PM
tzik updated this revision to Diff 151847.Jun 18 2018, 9:39 PM
tzik marked an inline comment as done.

drop an unneeded -O0

test/CodeGenCXX/nrvo-noopt.cpp
2

Thanks! Done.

This revision was automatically updated to reflect the committed changes.