[PATCH] D47586: Update NRVO logic to support early return (Attempt 2)

2018-06-18 Thread Taiju Tsuiki via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC335019: Update NRVO logic to support early return (Attempt 
2) (authored by tzik, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47586?vs=151847=151848#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47586

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/Scope.h
  lib/Sema/Scope.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenCXX/nrvo-noopt.cpp
  test/CodeGenCXX/nrvo.cpp
  test/SemaCXX/nrvo-ast.cpp

Index: include/clang/AST/Decl.h
===
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -879,6 +879,12 @@
 DAK_Normal
   };
 
+  enum NRVOMode {
+NRVO_Candidate,
+NRVO_Disabled,
+NRVO_Enabled,
+  };
+
   class ParmVarDeclBitfields {
 friend class ASTDeclReader;
 friend class ParmVarDecl;
@@ -931,7 +937,7 @@
 /// Whether this local variable could be allocated in the return
 /// slot of its function, enabling the named return value optimization
 /// (NRVO).
-unsigned NRVOVariable : 1;
+unsigned NRVOMode : 2;
 
 /// Whether this variable is the for-range-declaration in a C++0x
 /// for-range statement.
@@ -1319,12 +1325,20 @@
   /// return slot when returning from the function. Within the function body,
   /// each return that returns the NRVO object will have this variable as its
   /// NRVO candidate.
+  NRVOMode getNRVOMode() const {
+if (isa(this))
+  return NRVO_Disabled;
+return static_cast(NonParmVarDeclBits.NRVOMode);
+  }
+  bool isNRVOCandidate() const {
+return isa(this) ? false : NonParmVarDeclBits.NRVOMode == NRVO_Candidate;
+  }
   bool isNRVOVariable() const {
-return isa(this) ? false : NonParmVarDeclBits.NRVOVariable;
+return isa(this) ? false : NonParmVarDeclBits.NRVOMode == NRVO_Enabled;
   }
   void setNRVOVariable(bool NRVO) {
 assert(!isa(this));
-NonParmVarDeclBits.NRVOVariable = NRVO;
+NonParmVarDeclBits.NRVOMode = NRVO ? NRVO_Enabled : NRVO_Disabled;
   }
 
   /// Determine whether this variable is the for-range-declaration in
Index: include/clang/Sema/Scope.h
===
--- include/clang/Sema/Scope.h
+++ include/clang/Sema/Scope.h
@@ -201,10 +201,6 @@
   /// Used to determine if errors occurred in this scope.
   DiagnosticErrorTrap ErrorTrap;
 
-  /// A lattice consisting of undefined, a single NRVO candidate variable in
-  /// this scope, or over-defined. The bit is true when over-defined.
-  llvm::PointerIntPair NRVO;
-
   void setFlags(Scope *Parent, unsigned F);
 
 public:
@@ -466,23 +462,7 @@
   UsingDirectives.end());
   }
 
-  void addNRVOCandidate(VarDecl *VD) {
-if (NRVO.getInt())
-  return;
-if (NRVO.getPointer() == nullptr) {
-  NRVO.setPointer(VD);
-  return;
-}
-if (NRVO.getPointer() != VD)
-  setNoNRVO();
-  }
-
-  void setNoNRVO() {
-NRVO.setInt(true);
-NRVO.setPointer(nullptr);
-  }
-
-  void mergeNRVOIntoParent();
+  void setNRVOCandidate(VarDecl *Candidate);
 
   /// Init - This is used by the parser to implement scope caching.
   void Init(Scope *parent, unsigned flags);
Index: test/SemaCXX/nrvo-ast.cpp
===
--- test/SemaCXX/nrvo-ast.cpp
+++ test/SemaCXX/nrvo-ast.cpp
@@ -0,0 +1,153 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s
+
+struct X {
+  X();
+  X(const X&);
+  X(X&&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_00
+X test_00() {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_01
+X test_01(bool b) {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  if (b)
+return x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_02
+X test_02(bool b) {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo
+  X y;
+  if (b)
+return y;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_03
+X test_03(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+extern "C" _Noreturn void exit(int) throw();
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_04
+X test_04(bool b) {
+  {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+if (b)
+  return x;
+  }
+  exit(1);
+}
+
+void may_throw();
+// CHECK-LABEL: FunctionDecl {{.*}} test_05
+X test_05() {
+  try {
+may_throw();
+return X();
+  } catch (X x) {
+// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+return x;
+  }
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_06
+X test_06() {
+  // CHECK-NOT:

[PATCH] D47586: Update NRVO logic to support early return (Attempt 2)

2018-06-18 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added inline comments.



Comment at: test/CodeGenCXX/nrvo-noopt.cpp:1
+// RUN: %clang_cc1 -emit-llvm -O0 -o - %s | FileCheck %s
+

rsmith wrote:
> You don't need the `-O0` here; that's the default.
Thanks! Done.


Repository:
  rC Clang

https://reviews.llvm.org/D47586



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47586: Update NRVO logic to support early return (Attempt 2)

2018-06-18 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik updated this revision to Diff 151847.
tzik marked an inline comment as done.
tzik added a comment.

drop an unneeded -O0


Repository:
  rC Clang

https://reviews.llvm.org/D47586

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/Scope.h
  lib/Sema/Scope.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenCXX/nrvo-noopt.cpp
  test/CodeGenCXX/nrvo.cpp
  test/SemaCXX/nrvo-ast.cpp

Index: test/SemaCXX/nrvo-ast.cpp
===
--- /dev/null
+++ test/SemaCXX/nrvo-ast.cpp
@@ -0,0 +1,153 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s
+
+struct X {
+  X();
+  X(const X&);
+  X(X&&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_00
+X test_00() {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_01
+X test_01(bool b) {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  if (b)
+return x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_02
+X test_02(bool b) {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo
+  X y;
+  if (b)
+return y;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_03
+X test_03(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+extern "C" _Noreturn void exit(int) throw();
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_04
+X test_04(bool b) {
+  {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+if (b)
+  return x;
+  }
+  exit(1);
+}
+
+void may_throw();
+// CHECK-LABEL: FunctionDecl {{.*}} test_05
+X test_05() {
+  try {
+may_throw();
+return X();
+  } catch (X x) {
+// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+return x;
+  }
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_06
+X test_06() {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x __attribute__((aligned(8)));
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_07
+X test_07(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  }
+  return X();
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_08
+X test_08(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  } else {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+}
+
+template 
+struct Y {
+  Y();
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+  static Y test_09() {
+Y y;
+return y;
+  }
+};
+
+struct Z {
+  Z(const X&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'A ()'
+// CHECK: VarDecl {{.*}} b 'B' nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'X ()'
+// CHECK: VarDecl {{.*}} b {{.*}} nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'Z ()'
+// CHECK-NOT: VarDecl {{.*}} b {{.*}} nrvo
+template 
+A test_10() {
+  B b;
+  return b;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_11 'A (bool)'
+// CHECK-NOT: VarDecl {{.*}} a {{.*}} nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_11 'X (bool)'
+// CHECK-NOT: VarDecl {{.*}} a {{.*}} nrvo
+template 
+A test_11(bool b) {
+  A a;
+  if (b)
+return A();
+  return a;
+}
+
+void instantiate() {
+  Y::test_09();
+  test_10();
+  test_10();
+  test_11(true);
+}
Index: test/CodeGenCXX/nrvo.cpp
===
--- test/CodeGenCXX/nrvo.cpp
+++ test/CodeGenCXX/nrvo.cpp
@@ -130,17 +130,13 @@
 }
 
 // CHECK-LABEL: define void @_Z5test3b
-X test3(bool B) {
+X test3(bool B, X x) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
-  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
-  // CHECK: call {{.*}} @_ZN1XC1Ev
-  // CHECK: call {{.*}} @_ZN1XC1ERKS_
   if (B) {
 X y;
 return y;
   }
-  // FIXME: we should NRVO this variable too.
-  X x;
+  // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
   return x;
 }
 
@@ -191,21 +187,29 @@
 }
 
 // CHECK-LABEL: define void @_Z5test7b
+// CHECK-EH-LABEL: define void @_Z5test7b
 X test7(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
   if (b) {
 X x;
 return x;
   }
   return X();
 }
 
 // CHECK-LABEL: define void @_Z5test8b
+// CHECK-EH-LABEL: define void @_Z5test8b
 X test8(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
-  if (b) {
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
+if (b) {
 X x;
 return x;
   } else {
@@ -221,4 +225,37 @@
 // CHECK-LABEL: define linkonce_odr void @_ZN1YIiE1fEv
 // CHECK: tail call {{.*}} @_ZN1YIiEC1Ev
 
+// CHECK-LABEL: define void @_Z6test10b
+X test10(bool B, X x)

[PATCH] D47586: Update NRVO logic to support early return (Attempt 2)

2018-06-11 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added a comment.

rsmith: ping. Any chance you could review this?


Repository:
  rC Clang

https://reviews.llvm.org/D47586



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47586: Update NRVO logic to support early return (Attempt 2)

2018-05-31 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added a reviewer: rsmith.
tzik added a comment.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D47586



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47586: Update NRVO logic to support early return (Attempt 2)

2018-05-31 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik updated this revision to Diff 149299.
tzik added a comment.

test


Repository:
  rC Clang

https://reviews.llvm.org/D47586

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/Scope.h
  lib/Sema/Scope.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenCXX/nrvo-noopt.cpp
  test/CodeGenCXX/nrvo.cpp
  test/SemaCXX/nrvo-ast.cpp

Index: test/SemaCXX/nrvo-ast.cpp
===
--- /dev/null
+++ test/SemaCXX/nrvo-ast.cpp
@@ -0,0 +1,153 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s
+
+struct X {
+  X();
+  X(const X&);
+  X(X&&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_00
+X test_00() {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_01
+X test_01(bool b) {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  if (b)
+return x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_02
+X test_02(bool b) {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo
+  X y;
+  if (b)
+return y;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_03
+X test_03(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+extern "C" _Noreturn void exit(int) throw();
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_04
+X test_04(bool b) {
+  {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+if (b)
+  return x;
+  }
+  exit(1);
+}
+
+void may_throw();
+// CHECK-LABEL: FunctionDecl {{.*}} test_05
+X test_05() {
+  try {
+may_throw();
+return X();
+  } catch (X x) {
+// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+return x;
+  }
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_06
+X test_06() {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x __attribute__((aligned(8)));
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_07
+X test_07(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  }
+  return X();
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_08
+X test_08(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  } else {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+}
+
+template 
+struct Y {
+  Y();
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+  static Y test_09() {
+Y y;
+return y;
+  }
+};
+
+struct Z {
+  Z(const X&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'A ()'
+// CHECK: VarDecl {{.*}} b 'B' nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'X ()'
+// CHECK: VarDecl {{.*}} b {{.*}} nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'Z ()'
+// CHECK-NOT: VarDecl {{.*}} b {{.*}} nrvo
+template 
+A test_10() {
+  B b;
+  return b;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_11 'A (bool)'
+// CHECK-NOT: VarDecl {{.*}} a {{.*}} nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_11 'X (bool)'
+// CHECK-NOT: VarDecl {{.*}} a {{.*}} nrvo
+template 
+A test_11(bool b) {
+  A a;
+  if (b)
+return A();
+  return a;
+}
+
+void instantiate() {
+  Y::test_09();
+  test_10();
+  test_10();
+  test_11(true);
+}
Index: test/CodeGenCXX/nrvo.cpp
===
--- test/CodeGenCXX/nrvo.cpp
+++ test/CodeGenCXX/nrvo.cpp
@@ -130,17 +130,13 @@
 }
 
 // CHECK-LABEL: define void @_Z5test3b
-X test3(bool B) {
+X test3(bool B, X x) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
-  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
-  // CHECK: call {{.*}} @_ZN1XC1Ev
-  // CHECK: call {{.*}} @_ZN1XC1ERKS_
   if (B) {
 X y;
 return y;
   }
-  // FIXME: we should NRVO this variable too.
-  X x;
+  // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
   return x;
 }
 
@@ -191,21 +187,29 @@
 }
 
 // CHECK-LABEL: define void @_Z5test7b
+// CHECK-EH-LABEL: define void @_Z5test7b
 X test7(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
   if (b) {
 X x;
 return x;
   }
   return X();
 }
 
 // CHECK-LABEL: define void @_Z5test8b
+// CHECK-EH-LABEL: define void @_Z5test8b
 X test8(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
-  if (b) {
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
+if (b) {
 X x;
 return x;
   } else {
@@ -221,4 +225,37 @@
 // CHECK-LABEL: define linkonce_odr void @_ZN1YIiE1fEv
 // CHECK: tail call {{.*}} @_ZN1YIiEC1Ev
 
+// CHECK-LABEL: define void @_Z6test10b
+X test10(bool B, X x) {
+  if (B) {
+// CHECK: tail call {{.*}}

[PATCH] D47586: Update NRVO logic to support early return (Attempt 2)

2018-05-31 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik created this revision.
Herald added a subscriber: cfe-commits.

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 
  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.


Repository:
  rC Clang

https://reviews.llvm.org/D47586

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/Scope.h
  lib/Sema/Scope.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenCXX/nrvo-noopt.cpp
  test/CodeGenCXX/nrvo.cpp
  test/SemaCXX/nrvo-ast.cpp

Index: test/SemaCXX/nrvo-ast.cpp
===
--- /dev/null
+++ test/SemaCXX/nrvo-ast.cpp
@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s
+
+struct X {
+  X();
+  X(const X&);
+  X(X&&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_00
+X test_00() {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_01
+X test_01(bool b) {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  if (b)
+return x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_02
+X test_02(bool b) {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo
+  X y;
+  if (b)
+return y;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_03
+X test_03(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+extern "C" _Noreturn void exit(int) throw();
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_04
+X test_04(bool b) {
+  {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+if (b)
+  return x;
+  }
+  exit(1);
+}
+
+void may_throw();
+// CHECK-LABEL: FunctionDecl {{.*}} test_05
+X test_05() {
+  try {
+may_throw();
+return X();
+  } catch (X x) {
+// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+return x;
+  }
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_06
+X test_06() {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x __attribute__((aligned(8)));
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_07
+X test_07(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  }
+  return X();
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_08
+X test_08(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  } else {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+}
+
+template 
+struct Y {
+  Y();
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+  static Y test_09() {
+Y y;
+return y;
+  }
+};
+
+struct Z {
+  Z(const X&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'A ()'
+// CHECK: VarDecl {{.*}} b 'B' nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'X ()'
+// CHECK: VarDecl {{.*}} b {{.*}} nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'Z ()'
+// CHECK-NOT: VarDecl {{.*}} b {{.*}} nrvo
+template 
+A test_10() {
+  B b;
+  return b;
+}
+
+void instantiate() {
+  Y::test_09();
+  test_10();
+  test_10();
+}
Index: test/CodeGenCXX/nrvo.cpp
===
--- test/CodeGenCXX/nrvo.cpp
+++ test/CodeGenCXX/nrvo.cpp
@@ -130,17 +130,13 @@
 }
 
 // CHECK-LABEL: define void @_Z5test3b
-X test3(bool B) {
+X test3(bool B, X x) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
-  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
-  // CHECK: call {{.*}} @_ZN1XC1Ev
-  // CHECK: call {{.*}} @_ZN1XC1ERKS_
   if (B) {
 X y;
 return y;
   }
-  // FIXME: we should NRVO this variable too.
-  X x;
+  // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
   return x;
 }
 
@@ -191,21 +187,29 @@
 }
 
 // CHECK-LABEL: define void @_Z5test7b
+// CHECK-EH-LABEL: define void @_Z5test7b
 X test7(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
   if (b) {
 X x;
 return x;
   }
   return X();
 }
 
 // CHECK-LABEL: define void @_Z5test8b
+// CHECK-EH-LABEL: define void @_Z5test8b
 X test8(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
-  if (b) {
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
+if (b) {
 X x;
 return x;
   } else {
@@ -221,4 +225,37 @@
 // CHECK-LABEL: define linkonce_odr void @_ZN1YIiE1fEv
 //

[PATCH] D47067: Update NRVO logic to support early return

2018-05-31 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added a comment.

In https://reviews.llvm.org/D47067#1116733, @rsmith wrote:

> Slightly reduced testcase:
>
>   template T f(T u, bool b) {
> T t;
> if (b) return t;
> return u;
>   }
>   struct X { X(); X(const X&); ~X(); void *data; };
>  
>   template X f(X, bool);
>
>
> ... which compiles to:
>
>   X f(X u, bool b) {
> X::X();
> if (b) return;
> X::X(, u);
> X::~X();
>   }
>
>
> ... due to `t` incorrectly being used as an NRVO variable.


Thanks for your investigation!
I thought that's covered by Scope::setNRVOCandidate with a null `Candidate`. As 
`t` is not marked as an NRVO variable before its instantiation. There's likely 
something I missed that turn the flag on while its instantiation.


Repository:
  rC Clang

https://reviews.llvm.org/D47067



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47067: Update NRVO logic to support early return

2018-05-30 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added a comment.

In https://reviews.llvm.org/D47067#1116246, @sammccall wrote:

> Unfortunately this seems to miscompile, the stage1 built clang is crashing on 
> the multistage buildbots:
>
> http://lab.llvm.org:8011/builders/clang-s390x-linux-multistage/builds/2926 
> shows this crash present already at 333500
>  I've locally verified the crash and that that reverting this patch fixes it.
>
> I'm going to revert it.


Ah... Thanks for taking care.


Repository:
  rC Clang

https://reviews.llvm.org/D47067



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47067: Update NRVO logic to support early return

2018-05-29 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added a comment.

In https://reviews.llvm.org/D47067#1115566, @rsmith wrote:

> Thank you, do you need someone to commit this for you?


No, I recently got the commit access to the repository.


Repository:
  rC Clang

https://reviews.llvm.org/D47067



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47067: Update NRVO logic to support early return

2018-05-29 Thread Taiju Tsuiki via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC333500: Update NRVO logic to support early return (authored 
by tzik, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47067?vs=148603=149035#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47067

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/Scope.h
  lib/Sema/Scope.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenCXX/nrvo-noopt.cpp
  test/CodeGenCXX/nrvo.cpp
  test/SemaCXX/nrvo-ast.cpp

Index: include/clang/AST/Decl.h
===
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -879,6 +879,12 @@
 DAK_Normal
   };
 
+  enum NRVOMode {
+NRVO_Candidate,
+NRVO_Disabled,
+NRVO_Enabled,
+  };
+
   class ParmVarDeclBitfields {
 friend class ASTDeclReader;
 friend class ParmVarDecl;
@@ -931,7 +937,7 @@
 /// Whether this local variable could be allocated in the return
 /// slot of its function, enabling the named return value optimization
 /// (NRVO).
-unsigned NRVOVariable : 1;
+unsigned NRVOMode : 2;
 
 /// Whether this variable is the for-range-declaration in a C++0x
 /// for-range statement.
@@ -1319,12 +1325,20 @@
   /// return slot when returning from the function. Within the function body,
   /// each return that returns the NRVO object will have this variable as its
   /// NRVO candidate.
+  NRVOMode getNRVOMode() const {
+if (isa(this))
+  return NRVO_Disabled;
+return static_cast(NonParmVarDeclBits.NRVOMode);
+  }
+  bool isNRVOCandidate() const {
+return isa(this) ? false : NonParmVarDeclBits.NRVOMode == NRVO_Candidate;
+  }
   bool isNRVOVariable() const {
-return isa(this) ? false : NonParmVarDeclBits.NRVOVariable;
+return isa(this) ? false : NonParmVarDeclBits.NRVOMode == NRVO_Enabled;
   }
   void setNRVOVariable(bool NRVO) {
 assert(!isa(this));
-NonParmVarDeclBits.NRVOVariable = NRVO;
+NonParmVarDeclBits.NRVOMode = NRVO ? NRVO_Enabled : NRVO_Disabled;
   }
 
   /// Determine whether this variable is the for-range-declaration in
Index: include/clang/Sema/Scope.h
===
--- include/clang/Sema/Scope.h
+++ include/clang/Sema/Scope.h
@@ -201,10 +201,6 @@
   /// Used to determine if errors occurred in this scope.
   DiagnosticErrorTrap ErrorTrap;
 
-  /// A lattice consisting of undefined, a single NRVO candidate variable in
-  /// this scope, or over-defined. The bit is true when over-defined.
-  llvm::PointerIntPair NRVO;
-
   void setFlags(Scope *Parent, unsigned F);
 
 public:
@@ -466,23 +462,7 @@
   UsingDirectives.end());
   }
 
-  void addNRVOCandidate(VarDecl *VD) {
-if (NRVO.getInt())
-  return;
-if (NRVO.getPointer() == nullptr) {
-  NRVO.setPointer(VD);
-  return;
-}
-if (NRVO.getPointer() != VD)
-  setNoNRVO();
-  }
-
-  void setNoNRVO() {
-NRVO.setInt(true);
-NRVO.setPointer(nullptr);
-  }
-
-  void mergeNRVOIntoParent();
+  void setNRVOCandidate(VarDecl *Candidate);
 
   /// Init - This is used by the parser to implement scope caching.
   void Init(Scope *parent, unsigned flags);
Index: test/SemaCXX/nrvo-ast.cpp
===
--- test/SemaCXX/nrvo-ast.cpp
+++ test/SemaCXX/nrvo-ast.cpp
@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s
+
+struct X {
+  X();
+  X(const X&);
+  X(X&&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_00
+X test_00() {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_01
+X test_01(bool b) {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  if (b)
+return x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_02
+X test_02(bool b) {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo
+  X y;
+  if (b)
+return y;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_03
+X test_03(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+extern "C" _Noreturn void exit(int) throw();
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_04
+X test_04(bool b) {
+  {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+if (b)
+  return x;
+  }
+  exit(1);
+}
+
+void may_throw();
+// CHECK-LABEL: FunctionDecl {{.*}} test_05
+X test_05() {
+  try {
+may_throw();
+return X();
+  } catch (X x) {
+// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+return x;
+  }
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_06
+X test_06() {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x

[PATCH] D47067: Update NRVO logic to support early return

2018-05-25 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added inline comments.



Comment at: lib/Sema/Scope.cpp:128
 
-  if (getEntity())
-return;
-
-  if (NRVO.getInt())
-getParent()->setNoNRVO();
-  else if (NRVO.getPointer())
-getParent()->addNRVOCandidate(NRVO.getPointer());
+  if (getParent())
+getParent()->setNRVOCandidate(Candidate);

xbolva00 wrote:
> auto * Parent = getParent();
> if (Parent)
> Parent>setNRVOCandidate(Candidate);
> 
> ?
Updated!


Repository:
  rC Clang

https://reviews.llvm.org/D47067



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47067: Update NRVO logic to support early return

2018-05-25 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik updated this revision to Diff 148603.
tzik added a comment.

reuse getParent() result


Repository:
  rC Clang

https://reviews.llvm.org/D47067

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/Scope.h
  lib/Sema/Scope.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenCXX/nrvo-noopt.cpp
  test/CodeGenCXX/nrvo.cpp
  test/SemaCXX/nrvo-ast.cpp

Index: test/SemaCXX/nrvo-ast.cpp
===
--- /dev/null
+++ test/SemaCXX/nrvo-ast.cpp
@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s
+
+struct X {
+  X();
+  X(const X&);
+  X(X&&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_00
+X test_00() {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_01
+X test_01(bool b) {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  if (b)
+return x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_02
+X test_02(bool b) {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo
+  X y;
+  if (b)
+return y;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_03
+X test_03(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+extern "C" _Noreturn void exit(int) throw();
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_04
+X test_04(bool b) {
+  {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+if (b)
+  return x;
+  }
+  exit(1);
+}
+
+void may_throw();
+// CHECK-LABEL: FunctionDecl {{.*}} test_05
+X test_05() {
+  try {
+may_throw();
+return X();
+  } catch (X x) {
+// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+return x;
+  }
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_06
+X test_06() {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x __attribute__((aligned(8)));
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_07
+X test_07(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  }
+  return X();
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_08
+X test_08(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  } else {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+}
+
+template 
+struct Y {
+  Y();
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+  static Y test_09() {
+Y y;
+return y;
+  }
+};
+
+struct Z {
+  Z(const X&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'A ()'
+// CHECK: VarDecl {{.*}} b 'B' nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'X ()'
+// CHECK: VarDecl {{.*}} b {{.*}} nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'Z ()'
+// CHECK-NOT: VarDecl {{.*}} b {{.*}} nrvo
+template 
+A test_10() {
+  B b;
+  return b;
+}
+
+void instantiate() {
+  Y::test_09();
+  test_10();
+  test_10();
+}
Index: test/CodeGenCXX/nrvo.cpp
===
--- test/CodeGenCXX/nrvo.cpp
+++ test/CodeGenCXX/nrvo.cpp
@@ -130,17 +130,13 @@
 }
 
 // CHECK-LABEL: define void @_Z5test3b
-X test3(bool B) {
+X test3(bool B, X x) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
-  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
-  // CHECK: call {{.*}} @_ZN1XC1Ev
-  // CHECK: call {{.*}} @_ZN1XC1ERKS_
   if (B) {
 X y;
 return y;
   }
-  // FIXME: we should NRVO this variable too.
-  X x;
+  // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
   return x;
 }
 
@@ -191,21 +187,29 @@
 }
 
 // CHECK-LABEL: define void @_Z5test7b
+// CHECK-EH-LABEL: define void @_Z5test7b
 X test7(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
   if (b) {
 X x;
 return x;
   }
   return X();
 }
 
 // CHECK-LABEL: define void @_Z5test8b
+// CHECK-EH-LABEL: define void @_Z5test8b
 X test8(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
-  if (b) {
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
+if (b) {
 X x;
 return x;
   } else {
@@ -221,4 +225,37 @@
 // CHECK-LABEL: define linkonce_odr void @_ZN1YIiE1fEv
 // CHECK: tail call {{.*}} @_ZN1YIiEC1Ev
 
+// CHECK-LABEL: define void @_Z6test10b
+X test10(bool B, X x) {
+  if (B) {
+// CHECK: tail call {{.*}} @_ZN1XC1ERKS_
+// CHECK-EH: tail call {{.*}} @_ZN1XC1ERKS_
+return x;
+  }
+  // CHECK: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NOT: call {{.*}} @_ZN1XC1ERKS_
+  X y;
+  return y;
+}
+
+// CHECK-LABEL: define {{.*}} void

[PATCH] D47067: Update NRVO logic to support early return

2018-05-25 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added inline comments.



Comment at: lib/Sema/Scope.cpp:122-123
+void Scope::setNRVOCandidate(VarDecl *Candidate) {
+  for (auto* D : DeclsInScope) {
+VarDecl* VD = dyn_cast_or_null(D);
+if (VD && VD != Candidate && VD->isNRVOCandidate())

rsmith wrote:
> `*` on the right, please. Also `auto` -> `Decl` would be clearer and no 
> longer. Is `dyn_cast_or_null` really necessary? (Can `DeclsInScope` contain 
> `nullptr`?) I would expect that just `dyn_cast` would suffice.
Done! Right, contents in `DeclsInScope` should be non-null, and dyn_cast should 
work well.



Comment at: lib/Sema/SemaDecl.cpp:12619-12620
 void Sema::computeNRVO(Stmt *Body, FunctionScopeInfo *Scope) {
-  ReturnStmt **Returns = Scope->Returns.data();
+  for (ReturnStmt* Return : Scope->Returns) {
+const VarDecl* Candidate = Return->getNRVOCandidate();
+if (!Candidate)

rsmith wrote:
> `*` on the right, please.
Done


Repository:
  rC Clang

https://reviews.llvm.org/D47067



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47067: Update NRVO logic to support early return

2018-05-25 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik updated this revision to Diff 148570.
tzik marked 2 inline comments as done.
tzik added a comment.

style fix. -O0 IR test.


Repository:
  rC Clang

https://reviews.llvm.org/D47067

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/Scope.h
  lib/Sema/Scope.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenCXX/nrvo-noopt.cpp
  test/CodeGenCXX/nrvo.cpp
  test/SemaCXX/nrvo-ast.cpp

Index: test/SemaCXX/nrvo-ast.cpp
===
--- /dev/null
+++ test/SemaCXX/nrvo-ast.cpp
@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s
+
+struct X {
+  X();
+  X(const X&);
+  X(X&&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_00
+X test_00() {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_01
+X test_01(bool b) {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  if (b)
+return x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_02
+X test_02(bool b) {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo
+  X y;
+  if (b)
+return y;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_03
+X test_03(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+extern "C" _Noreturn void exit(int) throw();
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_04
+X test_04(bool b) {
+  {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+if (b)
+  return x;
+  }
+  exit(1);
+}
+
+void may_throw();
+// CHECK-LABEL: FunctionDecl {{.*}} test_05
+X test_05() {
+  try {
+may_throw();
+return X();
+  } catch (X x) {
+// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+return x;
+  }
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_06
+X test_06() {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x __attribute__((aligned(8)));
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_07
+X test_07(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  }
+  return X();
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_08
+X test_08(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  } else {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+}
+
+template 
+struct Y {
+  Y();
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+  static Y test_09() {
+Y y;
+return y;
+  }
+};
+
+struct Z {
+  Z(const X&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'A ()'
+// CHECK: VarDecl {{.*}} b 'B' nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'X ()'
+// CHECK: VarDecl {{.*}} b {{.*}} nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'Z ()'
+// CHECK-NOT: VarDecl {{.*}} b {{.*}} nrvo
+template 
+A test_10() {
+  B b;
+  return b;
+}
+
+void instantiate() {
+  Y::test_09();
+  test_10();
+  test_10();
+}
Index: test/CodeGenCXX/nrvo.cpp
===
--- test/CodeGenCXX/nrvo.cpp
+++ test/CodeGenCXX/nrvo.cpp
@@ -130,17 +130,13 @@
 }
 
 // CHECK-LABEL: define void @_Z5test3b
-X test3(bool B) {
+X test3(bool B, X x) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
-  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
-  // CHECK: call {{.*}} @_ZN1XC1Ev
-  // CHECK: call {{.*}} @_ZN1XC1ERKS_
   if (B) {
 X y;
 return y;
   }
-  // FIXME: we should NRVO this variable too.
-  X x;
+  // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
   return x;
 }
 
@@ -191,21 +187,29 @@
 }
 
 // CHECK-LABEL: define void @_Z5test7b
+// CHECK-EH-LABEL: define void @_Z5test7b
 X test7(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
   if (b) {
 X x;
 return x;
   }
   return X();
 }
 
 // CHECK-LABEL: define void @_Z5test8b
+// CHECK-EH-LABEL: define void @_Z5test8b
 X test8(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
-  if (b) {
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
+if (b) {
 X x;
 return x;
   } else {
@@ -221,4 +225,37 @@
 // CHECK-LABEL: define linkonce_odr void @_ZN1YIiE1fEv
 // CHECK: tail call {{.*}} @_ZN1YIiEC1Ev
 
+// CHECK-LABEL: define void @_Z6test10b
+X test10(bool B, X x) {
+  if (B) {
+// CHECK: tail call {{.*}} @_ZN1XC1ERKS_
+// CHECK-EH: tail call {{.*}} @_ZN1XC1ERKS_
+return x;
+  }
+  // CHECK: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NOT: call {{.*}} @_ZN1XC1ERKS_
+  X y;
+  return y;
+}
+

[PATCH] D47067: Update NRVO logic to support early return

2018-05-24 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:12760
   // to deduce an implicit return type.
-  if (FD->getReturnType()->isRecordType() &&
-  (!getLangOpts().CPlusPlus || !FD->isDependentContext()))
+  if (!FD->getReturnType()->isScalarType())
 computeNRVO(Body, getCurFunction());

Quuxplusone wrote:
> rsmith wrote:
> > tzik wrote:
> > > Quuxplusone wrote:
> > > > What is the purpose of this change?
> > > > If it's "no functional change" it should be done separately IMHO; if it 
> > > > is supposed to change codegen, then it needs some codegen tests. (From 
> > > > looking at the code: maybe this affects codegen on functions that 
> > > > return member function pointers by value?)
> > > I think the previous implementation was incorrect.
> > > Though computeNRVO clears ReturnStmt::NRVOCandidate when the 
> > > corresponding variable is not NRVO variable, CGStmt checks both of 
> > > ReturnStmt::NRVOCandidate and VarDecl::NRVOVariable anyway.
> > > So, computeNRVO took no effect in the previous code, and the absence of 
> > > computeNRVO here on function templates did not matter.
> > > Note that there was no chance to call computeNRVO on function template 
> > > elsewhere too.
> > > 
> > > OTOH in the new implementation, computeNRVO is necessary, and its absence 
> > > on function templates matters.
> > > 
> > > We can remove computeNRVO here separately as a NFC patch and readd the 
> > > new impl to the same place, but it's probably less readable, IMO.
> > What happens if we can't tell whether we have an NRVO candidate or not 
> > until instantiation:
> > 
> > ```
> > template X f() {
> >   T v;
> >   return v; // is an NRVO candidate if T is X, otherwise is not
> > }
> > ```
> > 
> > (I think this is not hard to handle: the dependent construct here can only 
> > affect whether you get NRVO at all, not which variable you perform NRVO on, 
> > but I want to check that it is handled properly.)
> Ah, so if I understand correctly — which I probably don't —...
> the parts of this diff related to `isRecordType()` make NRVO more reliable on 
> template functions? Because the existing code has lots of checks for 
> `isRecordType()` which means that they don't run for dependent 
> (as-yet-unknown) types, even if those types are going to turn out to be 
> record types occasionally?
> 
> I see that line 12838 runs `computeNRVO` unconditionally for *all* 
> ObjCMethodDecls, //even// ones with scalar return types. So maybe running 
> `computeNRVO` unconditionally right here would also be correct?
> Datapoint: I made a patch to do nothing but replace this condition with `if 
> (Body)` and to remove the similar condition guarding `computeNRVO` in 
> `Sema::ActOnBlockStmtExpr`. It passed the Clang test suite with flying 
> colors. I don't know if this means that the test suite is missing some tests, 
> or if it means that the conditions are unnecessary.
> 
> IMHO if you're changing this anyway, it would be nice to put the remaining 
> condition/optimization `if (getReturnType().isScalarType()) return;` inside 
> `computeNRVO` itself, instead of repeating that condition at every call site.
`v` here is marked as an NRVO variable in its function template AST, and that 
propagates to the instantiation if the return type is compatible.
https://github.com/llvm-mirror/clang/blob/9c82d4ff6015e4477e924c8a495a834c2fced29e/lib/Sema/SemaTemplateInstantiateDecl.cpp#L743



Comment at: lib/Sema/SemaDecl.cpp:12760
   // to deduce an implicit return type.
-  if (FD->getReturnType()->isRecordType() &&
-  (!getLangOpts().CPlusPlus || !FD->isDependentContext()))
+  if (!FD->getReturnType()->isScalarType())
 computeNRVO(Body, getCurFunction());

tzik wrote:
> Quuxplusone wrote:
> > rsmith wrote:
> > > tzik wrote:
> > > > Quuxplusone wrote:
> > > > > What is the purpose of this change?
> > > > > If it's "no functional change" it should be done separately IMHO; if 
> > > > > it is supposed to change codegen, then it needs some codegen tests. 
> > > > > (From looking at the code: maybe this affects codegen on functions 
> > > > > that return member function pointers by value?)
> > > > I think the previous implementation was incorrect.
> > > > Though computeNRVO clears ReturnStmt::NRVOCandidate when the 
> > > > corresponding variable is not NRVO variable, CGStmt checks both of 
> > > > ReturnStmt::NRVOCandidate and VarDecl::NRVOVariable anyway.
> > > > So, computeNRVO took no effect in the previous code, and the absence of 
> > > > computeNRVO here on function templates did not matter.
> > > > Note that there was no chance to call computeNRVO on function template 
> > > > elsewhere too.
> > > > 
> > > > OTOH in the new implementation, computeNRVO is necessary, and its 
> > > > absence on function templates matters.
> > > > 
> > > > We can remove computeNRVO here separately as a NFC patch and readd the 
> > >

[PATCH] D47067: Update NRVO logic to support early return

2018-05-24 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik updated this revision to Diff 148417.
tzik added a comment.

Add AST test. computeNRVO unconditionally.


Repository:
  rC Clang

https://reviews.llvm.org/D47067

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/Scope.h
  lib/Sema/Scope.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenCXX/nrvo.cpp
  test/SemaCXX/nrvo-ast.cpp

Index: test/SemaCXX/nrvo-ast.cpp
===
--- /dev/null
+++ test/SemaCXX/nrvo-ast.cpp
@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s
+
+struct X {
+  X();
+  X(const X&);
+  X(X&&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_00
+X test_00() {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_01
+X test_01(bool b) {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  if (b)
+return x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_02
+X test_02(bool b) {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo
+  X y;
+  if (b)
+return y;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_03
+X test_03(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+extern "C" _Noreturn void exit(int) throw();
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_04
+X test_04(bool b) {
+  {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+if (b)
+  return x;
+  }
+  exit(1);
+}
+
+void may_throw();
+// CHECK-LABEL: FunctionDecl {{.*}} test_05
+X test_05() {
+  try {
+may_throw();
+return X();
+  } catch (X x) {
+// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+return x;
+  }
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_06
+X test_06() {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x __attribute__((aligned(8)));
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_07
+X test_07(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  }
+  return X();
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_08
+X test_08(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  } else {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+}
+
+template 
+struct Y {
+  Y();
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+  static Y test_09() {
+Y y;
+return y;
+  }
+};
+
+struct Z {
+  Z(const X&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'A ()'
+// CHECK: VarDecl {{.*}} b 'B' nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'X ()'
+// CHECK: VarDecl {{.*}} b {{.*}} nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'Z ()'
+// CHECK-NOT: VarDecl {{.*}} b {{.*}} nrvo
+template 
+A test_10() {
+  B b;
+  return b;
+}
+
+void instantiate() {
+  Y::test_09();
+  test_10();
+  test_10();
+}
Index: test/CodeGenCXX/nrvo.cpp
===
--- test/CodeGenCXX/nrvo.cpp
+++ test/CodeGenCXX/nrvo.cpp
@@ -130,17 +130,13 @@
 }
 
 // CHECK-LABEL: define void @_Z5test3b
-X test3(bool B) {
+X test3(bool B, X x) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
-  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
-  // CHECK: call {{.*}} @_ZN1XC1Ev
-  // CHECK: call {{.*}} @_ZN1XC1ERKS_
   if (B) {
 X y;
 return y;
   }
-  // FIXME: we should NRVO this variable too.
-  X x;
+  // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
   return x;
 }
 
@@ -191,21 +187,29 @@
 }
 
 // CHECK-LABEL: define void @_Z5test7b
+// CHECK-EH-LABEL: define void @_Z5test7b
 X test7(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
   if (b) {
 X x;
 return x;
   }
   return X();
 }
 
 // CHECK-LABEL: define void @_Z5test8b
+// CHECK-EH-LABEL: define void @_Z5test8b
 X test8(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
-  if (b) {
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
+if (b) {
 X x;
 return x;
   } else {
@@ -221,4 +225,37 @@
 // CHECK-LABEL: define linkonce_odr void @_ZN1YIiE1fEv
 // CHECK: tail call {{.*}} @_ZN1YIiEC1Ev
 
+// CHECK-LABEL: define void @_Z6test10b
+X test10(bool B, X x) {
+  if (B) {
+// CHECK: tail call {{.*}} @_ZN1XC1ERKS_
+// CHECK-EH: tail call {{.*}} @_ZN1XC1ERKS_
+return x;
+  }
+  // CHECK: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NOT: call {{.*}} @_ZN1XC1ERKS_
+  X y;
+  return y;
+}
+
+// CHECK-LABEL: define {{.*}} void @_Z6test11I1XET_v

[PATCH] D47067: Update NRVO logic to support early return

2018-05-22 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:12760
   // to deduce an implicit return type.
-  if (FD->getReturnType()->isRecordType() &&
-  (!getLangOpts().CPlusPlus || !FD->isDependentContext()))
+  if (!FD->getReturnType()->isScalarType())
 computeNRVO(Body, getCurFunction());

Quuxplusone wrote:
> What is the purpose of this change?
> If it's "no functional change" it should be done separately IMHO; if it is 
> supposed to change codegen, then it needs some codegen tests. (From looking 
> at the code: maybe this affects codegen on functions that return member 
> function pointers by value?)
I think the previous implementation was incorrect.
Though computeNRVO clears ReturnStmt::NRVOCandidate when the corresponding 
variable is not NRVO variable, CGStmt checks both of ReturnStmt::NRVOCandidate 
and VarDecl::NRVOVariable anyway.
So, computeNRVO took no effect in the previous code, and the absence of 
computeNRVO here on function templates did not matter.
Note that there was no chance to call computeNRVO on function template 
elsewhere too.

OTOH in the new implementation, computeNRVO is necessary, and its absence on 
function templates matters.

We can remove computeNRVO here separately as a NFC patch and readd the new impl 
to the same place, but it's probably less readable, IMO.



Comment at: test/CodeGenCXX/nrvo.cpp:139
   }
-  // FIXME: we should NRVO this variable too.
-  X x;
+  // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
   return x;

Quuxplusone wrote:
> You've changed this function from testing one thing with a FIXME, to testing 
> a completely different thing.  Could you add your new code as a new function, 
> and leave the old FIXME alone until it's fixed?
> Alternatively, if your patch actually fixes the FIXME, could you just replace 
> the FIXME comment with a CHECK and leave the rest of this function alone?
My patch fixes the FIXME.
However, on the resulting code of NRVOing,
```
X y;
return y;
```
and
```
X x;
return x;
```
get to the same code and unified. And, the function is simplified as
```
X test3(bool B) {
  X x;
  return x;
}
```
without the if statement.
So, there will be nothing to CHECK left here if I leave the code as-is. I think 
that does not fit to the test scenario.



Comment at: test/CodeGenCXX/nrvo.cpp:254
+  T t;
+  return t;
+}

Quuxplusone wrote:
> Just for my own curiosity: this new test case is surely unaffected by this 
> patch, right?
Hm, this is not so important anymore. This was to check if NRVO is working with 
function templates on the existing code, as computeNRVO was not called on them. 
And also this covered a regression in a draft patch, that didn't work on 
function templates.


Repository:
  rC Clang

https://reviews.llvm.org/D47067



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47067: Update NRVO logic to support early return

2018-05-18 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik created this revision.
Herald added a subscriber: cfe-commits.

The previous implementation misses an opportunity to apply NRVO (Named Return 
Value

Optimization) below. That discourages user to write early return code.
--

struct Foo {};

Foo f(bool b) {

  if (b)
return Foo();
  Foo oo;
  return oo;

}
-

That is, we can/should apply RVO for a return statement if it refers a 
non-parameter local variable,
and the variable is referred by all return statements reachable from the 
variable declaration.
While, the previous implementation disables the RVO in a scope if there are 
multiple return
statements that refers different variables.

On the new algorithm, local variables are in NRVO_Candidate state at first, and 
a return
statement changes it to NRVO_Disabled for all visible variables but the return 
statement refers.
Then, at the end of the function AST traversal, NRVO is enabled for variables 
in NRVO_Candidate
state and refers from at least one return statement.


Repository:
  rC Clang

https://reviews.llvm.org/D47067

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/Scope.h
  lib/Sema/Scope.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaStmt.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenCXX/nrvo.cpp

Index: test/CodeGenCXX/nrvo.cpp
===
--- test/CodeGenCXX/nrvo.cpp
+++ test/CodeGenCXX/nrvo.cpp
@@ -130,17 +130,13 @@
 }
 
 // CHECK-LABEL: define void @_Z5test3b
-X test3(bool B) {
+X test3(bool B, X x) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
-  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
-  // CHECK: call {{.*}} @_ZN1XC1Ev
-  // CHECK: call {{.*}} @_ZN1XC1ERKS_
   if (B) {
 X y;
 return y;
   }
-  // FIXME: we should NRVO this variable too.
-  X x;
+  // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
   return x;
 }
 
@@ -222,3 +218,16 @@
 // CHECK: tail call {{.*}} @_ZN1YIiEC1Ev
 
 // CHECK-EH-03: attributes [[NR_NUW]] = { noreturn nounwind }
+
+
+// CHECK-LABEL: define void @_Z6test10b
+X test10(bool B, X x) {
+  if (B) {
+// CHECK: tail call {{.*}} @_ZN1XC1ERKS_
+return x;
+  }
+  // CHECK: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
+  X y;
+  return y;
+}
Index: lib/Serialization/ASTWriterDecl.cpp
===
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -918,7 +918,7 @@
   if (!isa(D)) {
 Record.push_back(D->isThisDeclarationADemotedDefinition());
 Record.push_back(D->isExceptionVariable());
-Record.push_back(D->isNRVOVariable());
+Record.push_back(D->getNRVOMode());
 Record.push_back(D->isCXXForRangeDecl());
 Record.push_back(D->isObjCForDecl());
 Record.push_back(D->isARCPseudoStrong());
@@ -2031,7 +2031,7 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // InitStyle
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsThisDeclarationADemotedDefinition
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isExceptionVariable
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isNRVOVariable
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // NRVOMode
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isCXXForRangeDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isObjCForDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1326,7 +1326,7 @@
 VD->NonParmVarDeclBits.IsThisDeclarationADemotedDefinition =
 Record.readInt();
 VD->NonParmVarDeclBits.ExceptionVar = Record.readInt();
-VD->NonParmVarDeclBits.NRVOVariable = Record.readInt();
+VD->NonParmVarDeclBits.NRVOMode = Record.readInt();
 VD->NonParmVarDeclBits.CXXForRangeDecl = Record.readInt();
 VD->NonParmVarDeclBits.ObjCForDecl = Record.readInt();
 VD->NonParmVarDeclBits.ARCPseudoStrong = Record.readInt();
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -3455,12 +3455,9 @@
ExpressionEvaluationContext::DiscardedStatement)
 return R;
 
-  if (VarDecl *VD =
-  const_cast(cast(R.get())->getNRVOCandidate())) {
-CurScope->addNRVOCandidate(VD);
-  } else {
-CurScope->setNoNRVO();
-  }
+  VarDecl *VD =
+  const_cast(cast(R.get())->getNRVOCandidate());
+  CurScope->setNRVOCandidate(VD);
 
   CheckJumpOutOfSEHFinally(*this, ReturnLoc, *CurScope->getFnParent());
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -1798,8

[PATCH] D46929: Fix a mangling failure on clang-cl C++17

2018-05-17 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added a comment.

In https://reviews.llvm.org/D46929#1101780, @rnk wrote:

> I searched around, and I noticed that `VTableContext::getMethodVTableIndex` 
> has the same implied contract that the caller must always provide a canonical 
> declaration or things will break. It looks like it has three callers, and 
> they all do this. We should probably sink the canonicalization into this 
> helper as well and clean up the now-superfluous canonicalizations at the call 
> site.


Updated.
As I'm not sure if it's safe to use non-canonicalized CXXMethodDecl for 
EmitVirtualMemPtrThunk and CGCall, I left using the canonical decl.
For other part, non-canonical decl looks working fine to me.


Repository:
  rC Clang

https://reviews.llvm.org/D46929



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46929: Fix a mangling failure on clang-cl C++17

2018-05-16 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik updated this revision to Diff 147244.

Repository:
  rC Clang

https://reviews.llvm.org/D46929

Files:
  lib/AST/VTableBuilder.cpp
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  test/CodeGenCXX/PR37481.cpp

Index: test/CodeGenCXX/PR37481.cpp
===
--- /dev/null
+++ test/CodeGenCXX/PR37481.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -o /dev/null -emit-llvm -std=c++17 -triple x86_64-pc-windows-msvc %s
+
+struct Foo {
+  virtual void f();
+  virtual void g();
+};
+
+void Foo::f() {}
+void Foo::g() {}
+
+template 
+void h() {}
+
+void x() {
+  h<::f>();
+  h<::g>();
+}
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -228,7 +228,6 @@
 
   const CXXRecordDecl *
   getThisArgumentTypeForMethod(const CXXMethodDecl *MD) override {
-MD = MD->getCanonicalDecl();
 if (MD->isVirtual() && !isa(MD)) {
   MethodVFTableLocation ML =
   CGM.getMicrosoftVTableContext().getMethodVFTableLocation(MD);
@@ -1320,23 +1319,21 @@
 
 CharUnits
 MicrosoftCXXABI::getVirtualFunctionPrologueThisAdjustment(GlobalDecl GD) {
-  GD = GD.getCanonicalDecl();
   const CXXMethodDecl *MD = cast(GD.getDecl());
 
-  GlobalDecl LookupGD = GD;
   if (const CXXDestructorDecl *DD = dyn_cast(MD)) {
 // Complete destructors take a pointer to the complete object as a
 // parameter, thus don't need this adjustment.
 if (GD.getDtorType() == Dtor_Complete)
   return CharUnits();
 
 // There's no Dtor_Base in vftable but it shares the this adjustment with
 // the deleting one, so look it up instead.
-LookupGD = GlobalDecl(DD, Dtor_Deleting);
+GD = GlobalDecl(DD, Dtor_Deleting);
   }
 
   MethodVFTableLocation ML =
-  CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
+  CGM.getMicrosoftVTableContext().getMethodVFTableLocation(GD);
   CharUnits Adjustment = ML.VFPtrOffset;
 
   // Normal virtual instance methods need to adjust from the vfptr that first
@@ -1370,7 +1367,6 @@
 return CGF.Builder.CreateConstByteGEP(This, Adjustment);
   }
 
-  GD = GD.getCanonicalDecl();
   const CXXMethodDecl *MD = cast(GD.getDecl());
 
   GlobalDecl LookupGD = GD;
@@ -1839,7 +1835,6 @@
 Address This,
 llvm::Type *Ty,
 SourceLocation Loc) {
-  GD = GD.getCanonicalDecl();
   CGBuilderTy  = CGF.Builder;
 
   Ty = Ty->getPointerTo()->getPointerTo();
@@ -1878,7 +1873,7 @@
 VFunc = Builder.CreateAlignedLoad(VFuncPtr, CGF.getPointerAlign());
   }
 
-  CGCallee Callee(MethodDecl, VFunc);
+  CGCallee Callee(MethodDecl->getCanonicalDecl(), VFunc);
   return Callee;
 }
 
@@ -2737,7 +2732,6 @@
 MicrosoftCXXABI::EmitMemberFunctionPointer(const CXXMethodDecl *MD) {
   assert(MD->isInstance() && "Member function must not be static!");
 
-  MD = MD->getCanonicalDecl();
   CharUnits NonVirtualBaseAdjustment = CharUnits::Zero();
   const CXXRecordDecl *RD = MD->getParent()->getMostRecentDecl();
   CodeGenTypes  = CGM.getTypes();
@@ -2760,7 +2754,7 @@
   } else {
 auto  = CGM.getMicrosoftVTableContext();
 MethodVFTableLocation ML = VTableContext.getMethodVFTableLocation(MD);
-FirstField = EmitVirtualMemPtrThunk(MD, ML);
+FirstField = EmitVirtualMemPtrThunk(MD->getCanonicalDecl(), ML);
 // Include the vfptr adjustment if the method is in a non-primary vftable.
 NonVirtualBaseAdjustment += ML.VFPtrOffset;
 if (ML.VBase)
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -825,7 +825,6 @@
 llvm::Constant *ItaniumCXXABI::BuildMemberPointer(const CXXMethodDecl *MD,
   CharUnits ThisAdjustment) {
   assert(MD->isInstance() && "Member function must not be static!");
-  MD = MD->getCanonicalDecl();
 
   CodeGenTypes  = CGM.getTypes();
 
@@ -1640,7 +1639,6 @@
   Address This,
   llvm::Type *Ty,
   SourceLocation Loc) {
-  GD = GD.getCanonicalDecl();
   Ty = Ty->getPointerTo()->getPointerTo();
   auto *MethodDecl = cast(GD.getDecl());
   llvm::Value *VTable = CGF.GetVTablePtr(This, Ty, MethodDecl->getParent());
@@ -1674,7 +1672,7 @@
 VFunc = VFuncLoad;
   }
 
-  CGCallee Callee(MethodDecl, VFunc);
+  CGCallee Callee(MethodDecl->getCanonicalDecl(), VFunc);
   return Callee;
 }
 
Index: lib/CodeGen/CGCXX.cpp
===
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -267,7 +267,6 @@

[PATCH] D46820: Fix __uuidof handling on non-type template parameter in C++17

2018-05-16 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik marked an inline comment as done.
tzik added inline comments.



Comment at: test/SemaCXX/PR24986.cpp:12
+  f<&__uuidof(0)>();
+}

thakis wrote:
> Maybe this test could be in test/SemaCXX/ms-uuid.cpp instead?
OK, moved there.


Repository:
  rC Clang

https://reviews.llvm.org/D46820



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46820: Fix __uuidof handling on non-type template parameter in C++17

2018-05-16 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik updated this revision to Diff 147238.

Repository:
  rC Clang

https://reviews.llvm.org/D46820

Files:
  lib/Sema/SemaTemplate.cpp
  test/SemaCXX/ms-uuid.cpp


Index: test/SemaCXX/ms-uuid.cpp
===
--- test/SemaCXX/ms-uuid.cpp
+++ test/SemaCXX/ms-uuid.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions %s 
-Wno-deprecated-declarations
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify -fms-extensions %s 
-Wno-deprecated-declarations
 
 typedef struct _GUID {
   unsigned long Data1;
@@ -92,4 +93,16 @@
 // the previous case).
 [uuid("00A0---C000-0049"),
  uuid("00A0---C000-0049")] class C10;
+
+template 
+void F1() {
+  // Regression test for PR24986. The given GUID should just work as a pointer.
+  const GUID* q = p;
+}
+
+void F2() {
+  // The UUID should work for a non-type template parameter.
+  F1<&__uuidof(C1)>();
+}
+
 }
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6245,7 +6245,7 @@
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(const_cast(E));
+  Converted = TemplateArgument(ArgResult.get());
   break;
 }
 Diag(Arg->getLocStart(), diag::err_template_arg_not_decl_ref)


Index: test/SemaCXX/ms-uuid.cpp
===
--- test/SemaCXX/ms-uuid.cpp
+++ test/SemaCXX/ms-uuid.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions %s -Wno-deprecated-declarations
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify -fms-extensions %s -Wno-deprecated-declarations
 
 typedef struct _GUID {
   unsigned long Data1;
@@ -92,4 +93,16 @@
 // the previous case).
 [uuid("00A0---C000-0049"),
  uuid("00A0---C000-0049")] class C10;
+
+template 
+void F1() {
+  // Regression test for PR24986. The given GUID should just work as a pointer.
+  const GUID* q = p;
+}
+
+void F2() {
+  // The UUID should work for a non-type template parameter.
+  F1<&__uuidof(C1)>();
+}
+
 }
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6245,7 +6245,7 @@
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(const_cast(E));
+  Converted = TemplateArgument(ArgResult.get());
   break;
 }
 Diag(Arg->getLocStart(), diag::err_template_arg_not_decl_ref)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46820: Fix __uuidof handling on non-type template parameter in C++17

2018-05-16 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added a comment.

rsmith: Could you PTAL to this?


Repository:
  rC Clang

https://reviews.llvm.org/D46820



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46929: Fix a mangling failure on clang-cl C++17

2018-05-16 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik created this revision.
tzik added reviewers: majnemer, rnk.
Herald added a subscriber: cfe-commits.

MethodVFTableLocations in MigrosoftVTableContext contains canonicalized
decl. But, it's sometimes asked to lookup for non-canonicalized decl,
and that causes assertion failure, and compilation failure.
Fixes PR37481.


Repository:
  rC Clang

https://reviews.llvm.org/D46929

Files:
  lib/AST/VTableBuilder.cpp
  test/CodeGenCXX/PR37481.cpp


Index: test/CodeGenCXX/PR37481.cpp
===
--- /dev/null
+++ test/CodeGenCXX/PR37481.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -o /dev/null -emit-llvm -std=c++17 -triple 
x86_64-pc-windows-msvc %s
+
+struct Foo {
+  virtual void f();
+  virtual void g();
+};
+
+void Foo::f() {}
+void Foo::g() {}
+
+template 
+void h() {}
+
+void x() {
+  h<::f>();
+  h<::g>();
+}
Index: lib/AST/VTableBuilder.cpp
===
--- lib/AST/VTableBuilder.cpp
+++ lib/AST/VTableBuilder.cpp
@@ -3737,6 +3737,8 @@
   if (isa(GD.getDecl()))
 assert(GD.getDtorType() == Dtor_Deleting);
 
+  GD = GD.getCanonicalDecl();
+
   MethodVFTableLocationsTy::iterator I = MethodVFTableLocations.find(GD);
   if (I != MethodVFTableLocations.end())
 return I->second;


Index: test/CodeGenCXX/PR37481.cpp
===
--- /dev/null
+++ test/CodeGenCXX/PR37481.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -o /dev/null -emit-llvm -std=c++17 -triple x86_64-pc-windows-msvc %s
+
+struct Foo {
+  virtual void f();
+  virtual void g();
+};
+
+void Foo::f() {}
+void Foo::g() {}
+
+template 
+void h() {}
+
+void x() {
+  h<::f>();
+  h<::g>();
+}
Index: lib/AST/VTableBuilder.cpp
===
--- lib/AST/VTableBuilder.cpp
+++ lib/AST/VTableBuilder.cpp
@@ -3737,6 +3737,8 @@
   if (isa(GD.getDecl()))
 assert(GD.getDtorType() == Dtor_Deleting);
 
+  GD = GD.getCanonicalDecl();
+
   MethodVFTableLocationsTy::iterator I = MethodVFTableLocations.find(GD);
   if (I != MethodVFTableLocations.end())
 return I->second;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46820: Fix __uuidof handling on non-type template parameter in C++17

2018-05-14 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik created this revision.
tzik added reviewers: rsmith, thakis.
Herald added a subscriber: cfe-commits.

Clang used to pass the base lvalue of a non-type template parameter
to the template instantiation phase when the base part is __uuidof
and it's running in C++17 mode.
However, that drops its LValuePath, and unintentionally transforms
`&__uuidof(...)` to `__uuidof(...)`.

This CL fixes that by passing whole expr. Fixes PR24986.


Repository:
  rC Clang

https://reviews.llvm.org/D46820

Files:
  lib/Sema/SemaTemplate.cpp
  test/SemaCXX/PR24986.cpp


Index: test/SemaCXX/PR24986.cpp
===
--- /dev/null
+++ test/SemaCXX/PR24986.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -std=c++17 %s
+
+typedef struct _GUID {} GUID;
+
+template 
+void f() {
+  const GUID* q = p;
+}
+
+void g() {
+  f<&__uuidof(0)>();
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6197,7 +6197,7 @@
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(const_cast(E));
+  Converted = TemplateArgument(ArgResult.get());
   break;
 }
 Diag(Arg->getLocStart(), diag::err_template_arg_not_decl_ref)


Index: test/SemaCXX/PR24986.cpp
===
--- /dev/null
+++ test/SemaCXX/PR24986.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -std=c++17 %s
+
+typedef struct _GUID {} GUID;
+
+template 
+void f() {
+  const GUID* q = p;
+}
+
+void g() {
+  f<&__uuidof(0)>();
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6197,7 +6197,7 @@
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(const_cast(E));
+  Converted = TemplateArgument(ArgResult.get());
   break;
 }
 Diag(Arg->getLocStart(), diag::err_template_arg_not_decl_ref)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46385: Fix test failure for missing _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS

2018-05-03 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik created this revision.
tzik added a reviewer: thakis.
Herald added subscribers: cfe-commits, christof.

This is a follow-up change to r331150. The CL moved the macro from individual
file to build file, but the macro is missed in a test config file.


Repository:
  rCXXA libc++abi

https://reviews.llvm.org/D46385

Files:
  test/libcxxabi/test/config.py


Index: test/libcxxabi/test/config.py
===
--- test/libcxxabi/test/config.py
+++ test/libcxxabi/test/config.py
@@ -49,7 +49,10 @@
 self.config.available_features.add('libcxxabi-has-system-unwinder')
 
 def configure_compile_flags(self):
-self.cxx.compile_flags += ['-DLIBCXXABI_NO_TIMER']
+self.cxx.compile_flags += [
+'-DLIBCXXABI_NO_TIMER',
+'-D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS',
+]
 if self.get_lit_bool('enable_exceptions', True):
 self.cxx.compile_flags += ['-funwind-tables']
 else:


Index: test/libcxxabi/test/config.py
===
--- test/libcxxabi/test/config.py
+++ test/libcxxabi/test/config.py
@@ -49,7 +49,10 @@
 self.config.available_features.add('libcxxabi-has-system-unwinder')
 
 def configure_compile_flags(self):
-self.cxx.compile_flags += ['-DLIBCXXABI_NO_TIMER']
+self.cxx.compile_flags += [
+'-DLIBCXXABI_NO_TIMER',
+'-D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS',
+]
 if self.get_lit_bool('enable_exceptions', True):
 self.cxx.compile_flags += ['-funwind-tables']
 else:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46056: Move _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS macro to build system

2018-04-29 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added a comment.

rsmith: Thanks! I don't have the commit access to the repository. Could you 
submit this for me?


Repository:
  rCXXA libc++abi

https://reviews.llvm.org/D46056



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46056: Move _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS macro to build system

2018-04-25 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added a comment.

rsmith: Could you PTAL to this? This is an attempt to resolve 
https://bugs.llvm.org/show_bug.cgi?id=34103.
Though we already have a way to bring back std::unexpected to libc++abi, we 
currently don't apply it properly, IMO.


Repository:
  rCXXA libc++abi

https://reviews.llvm.org/D46056



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46056: Move _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS macro to build system

2018-04-25 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik created this revision.
tzik added reviewers: rsmith, thakis.
Herald added subscribers: cfe-commits, mgorny.

_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS is currently used to
bring back std::unexpected, which is removed in C++17, but still needed
for libc++abi for backward compatibility.

This macro used to define in cxa_exception.cpp only, but actually
needed for all sources that touches exceptions.
So, a build-system-level macro is better fit to define this macro.


Repository:
  rCXXA libc++abi

https://reviews.llvm.org/D46056

Files:
  CMakeLists.txt
  src/cxa_exception.cpp
  test/test_exception_storage.pass.cpp


Index: test/test_exception_storage.pass.cpp
===
--- test/test_exception_storage.pass.cpp
+++ test/test_exception_storage.pass.cpp
@@ -7,11 +7,6 @@
 //
 
//===--===//
 
-// FIXME: cxa_exception.hpp directly references `std::unexpected` and friends.
-// This breaks this test when compiled in C++17. For now fix this by manually
-// re-enabling the STL functions.
-#define _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS
-
 #include 
 #include 
 #include 
Index: src/cxa_exception.cpp
===
--- src/cxa_exception.cpp
+++ src/cxa_exception.cpp
@@ -11,8 +11,6 @@
 //  
 
//===--===//
 
-#define _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS
-
 #include "cxxabi.h"
 
 #include // for std::terminate
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -387,6 +387,10 @@
 # Prevent libc++abi from having library dependencies on libc++
 add_definitions(-D_LIBCPP_DISABLE_EXTERN_TEMPLATE)
 
+# Bring back `std::unexpected`, which is removed in C++17, to support
+# pre-C++17.
+add_definitions(-D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS)
+
 if (MSVC)
   add_definitions(-D_CRT_SECURE_NO_WARNINGS)
 endif()


Index: test/test_exception_storage.pass.cpp
===
--- test/test_exception_storage.pass.cpp
+++ test/test_exception_storage.pass.cpp
@@ -7,11 +7,6 @@
 //
 //===--===//
 
-// FIXME: cxa_exception.hpp directly references `std::unexpected` and friends.
-// This breaks this test when compiled in C++17. For now fix this by manually
-// re-enabling the STL functions.
-#define _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS
-
 #include 
 #include 
 #include 
Index: src/cxa_exception.cpp
===
--- src/cxa_exception.cpp
+++ src/cxa_exception.cpp
@@ -11,8 +11,6 @@
 //  
 //===--===//
 
-#define _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS
-
 #include "cxxabi.h"
 
 #include // for std::terminate
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -387,6 +387,10 @@
 # Prevent libc++abi from having library dependencies on libc++
 add_definitions(-D_LIBCPP_DISABLE_EXTERN_TEMPLATE)
 
+# Bring back `std::unexpected`, which is removed in C++17, to support
+# pre-C++17.
+add_definitions(-D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS)
+
 if (MSVC)
   add_definitions(-D_CRT_SECURE_NO_WARNINGS)
 endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33875: PR27037: Use correct CVR qualifier on an upcast on method pointer call

2017-06-06 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik marked an inline comment as done.
tzik added a comment.

In https://reviews.llvm.org/D33875#774293, @rsmith wrote:

> Looks good to me, thanks! Do you need someone to commit this for you?


Yes, could you commit this?


https://reviews.llvm.org/D33875



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33875: PR27037: Use correct CVR qualifier on an upcast on method pointer call

2017-06-06 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik marked an inline comment as done.
tzik added inline comments.



Comment at: lib/Sema/SemaExprCXX.cpp:5108
 QualType UseType = isIndirect ? Context.getPointerType(Class) : Class;
+UseType = 
UseType.withCVRQualifiers(LHS.get()->getType().getCVRQualifiers());
 ExprValueKind VK = isIndirect ? VK_RValue : LHS.get()->getValueKind();

rsmith wrote:
> In the "indirect" case, the cv-qualifiers should be taken from the pointee 
> type of the LHS and applied to the pointee type of UseType. I believe this 
> patch will not be enough to cause us to reject the indirect version of your 
> testcase:
> 
> ```
>   (()->*::f)();  // expected-error{{drops 'const' qualifier}}
> ```
> 
> Moreover, we should be preserving all qualifiers, not just cvr-qualifiers; 
> for example, this should also preserve the address space.
Make sense. OK, I updated the CL to cover that cases.
Could you take another look?


https://reviews.llvm.org/D33875



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33875: PR27037: Use correct CVR qualifier on an upcast on method pointer call

2017-06-06 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik updated this revision to Diff 101536.
tzik added a comment.

Cover indirect case and non-CVR qualifiers


https://reviews.llvm.org/D33875

Files:
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/PR27037.cpp


Index: test/SemaCXX/PR27037.cpp
===
--- /dev/null
+++ test/SemaCXX/PR27037.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct A {
+  void f();
+};
+
+struct B : A {};
+
+void m() {
+  const B b;
+  (b.*::f)();  // expected-error{{drops 'const' qualifier}}
+  (()->*::f)();  // expected-error{{drops 'const' qualifier}}
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -5104,7 +5104,9 @@
   return QualType();
 
 // Cast LHS to type of use.
-QualType UseType = isIndirect ? Context.getPointerType(Class) : Class;
+QualType UseType = Context.getQualifiedType(Class, 
LHSType.getQualifiers());
+if (isIndirect)
+  UseType = Context.getPointerType(UseType);
 ExprValueKind VK = isIndirect ? VK_RValue : LHS.get()->getValueKind();
 LHS = ImpCastExprToType(LHS.get(), UseType, CK_DerivedToBase, VK,
 );


Index: test/SemaCXX/PR27037.cpp
===
--- /dev/null
+++ test/SemaCXX/PR27037.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct A {
+  void f();
+};
+
+struct B : A {};
+
+void m() {
+  const B b;
+  (b.*::f)();  // expected-error{{drops 'const' qualifier}}
+  (()->*::f)();  // expected-error{{drops 'const' qualifier}}
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -5104,7 +5104,9 @@
   return QualType();
 
 // Cast LHS to type of use.
-QualType UseType = isIndirect ? Context.getPointerType(Class) : Class;
+QualType UseType = Context.getQualifiedType(Class, LHSType.getQualifiers());
+if (isIndirect)
+  UseType = Context.getPointerType(UseType);
 ExprValueKind VK = isIndirect ? VK_RValue : LHS.get()->getValueKind();
 LHS = ImpCastExprToType(LHS.get(), UseType, CK_DerivedToBase, VK,
 );
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33875: PR27037: Use correct CVR qualifier on an upcast on method pointer call

2017-06-05 Thread Taiju Tsuiki via Phabricator via cfe-commits 
tzik added a comment.

Hi, Richard.
Could you PTAL to this?


https://reviews.llvm.org/D33875



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits