[PATCH] D63157: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue conversion applied to a member access or similar not-quite-trivial lvalue expression.
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363295: C++ DR712 and others: handle non-odr-use resulting
from an lvalue-to-rvalue… (authored by rsmith, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D63157?vs=204589=204599#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63157/new/
https://reviews.llvm.org/D63157
Files:
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/CXX/basic/basic.def.odr/p2.cpp
cfe/trunk/test/CXX/drs/dr20xx.cpp
cfe/trunk/test/CXX/drs/dr21xx.cpp
cfe/trunk/test/CXX/drs/dr23xx.cpp
cfe/trunk/test/CXX/drs/dr6xx.cpp
cfe/trunk/test/CXX/drs/dr7xx.cpp
cfe/trunk/test/CodeGenCXX/no-odr-use.cpp
cfe/trunk/www/cxx_dr_status.html
Index: cfe/trunk/lib/CodeGen/CGDecl.cpp
===
--- cfe/trunk/lib/CodeGen/CGDecl.cpp
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp
@@ -1077,17 +1077,16 @@
return constant;
}
-static Address createUnnamedGlobalFrom(CodeGenModule , const VarDecl ,
- CGBuilderTy ,
- llvm::Constant *Constant,
- CharUnits Align) {
+Address CodeGenModule::createUnnamedGlobalFrom(const VarDecl ,
+ llvm::Constant *Constant,
+ CharUnits Align) {
auto FunctionName = [&](const DeclContext *DC) -> std::string {
if (const auto *FD = dyn_cast(DC)) {
if (const auto *CC = dyn_cast(FD))
return CC->getNameAsString();
if (const auto *CD = dyn_cast(FD))
return CD->getNameAsString();
- return CGM.getMangledName(FD);
+ return getMangledName(FD);
} else if (const auto *OM = dyn_cast(DC)) {
return OM->getNameAsString();
} else if (isa(DC)) {
@@ -1099,22 +1098,39 @@
}
};
- auto *Ty = Constant->getType();
- bool isConstant = true;
- llvm::GlobalVariable *InsertBefore = nullptr;
- unsigned AS = CGM.getContext().getTargetAddressSpace(
- CGM.getStringLiteralAddressSpace());
- llvm::GlobalVariable *GV = new llvm::GlobalVariable(
- CGM.getModule(), Ty, isConstant, llvm::GlobalValue::PrivateLinkage,
- Constant,
- "__const." + FunctionName(D.getParentFunctionOrMethod()) + "." +
- D.getName(),
- InsertBefore, llvm::GlobalValue::NotThreadLocal, AS);
- GV->setAlignment(Align.getQuantity());
- GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
-
- Address SrcPtr = Address(GV, Align);
- llvm::Type *BP = llvm::PointerType::getInt8PtrTy(CGM.getLLVMContext(), AS);
+ // Form a simple per-variable cache of these values in case we find we
+ // want to reuse them.
+ llvm::GlobalVariable * = InitializerConstants[];
+ if (!CacheEntry || CacheEntry->getInitializer() != Constant) {
+auto *Ty = Constant->getType();
+bool isConstant = true;
+llvm::GlobalVariable *InsertBefore = nullptr;
+unsigned AS =
+getContext().getTargetAddressSpace(getStringLiteralAddressSpace());
+llvm::GlobalVariable *GV = new llvm::GlobalVariable(
+getModule(), Ty, isConstant, llvm::GlobalValue::PrivateLinkage,
+Constant,
+"__const." + FunctionName(D.getParentFunctionOrMethod()) + "." +
+D.getName(),
+InsertBefore, llvm::GlobalValue::NotThreadLocal, AS);
+GV->setAlignment(Align.getQuantity());
+GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+CacheEntry = GV;
+ } else if (CacheEntry->getAlignment() < Align.getQuantity()) {
+CacheEntry->setAlignment(Align.getQuantity());
+ }
+
+ return Address(CacheEntry, Align);
+}
+
+static Address createUnnamedGlobalForMemcpyFrom(CodeGenModule ,
+const VarDecl ,
+CGBuilderTy ,
+llvm::Constant *Constant,
+CharUnits Align) {
+ Address SrcPtr = CGM.createUnnamedGlobalFrom(D, Constant, Align);
+ llvm::Type *BP = llvm::PointerType::getInt8PtrTy(CGM.getLLVMContext(),
+ SrcPtr.getAddressSpace());
if (SrcPtr.getType() != BP)
SrcPtr = Builder.CreateBitCast(SrcPtr, BP);
return SrcPtr;
@@ -1197,10 +1213,10 @@
}
// Copy from a global.
- Builder.CreateMemCpy(
- Loc,
- createUnnamedGlobalFrom(CGM, D, Builder, constant, Loc.getAlignment()),
- SizeVal, isVolatile);
+ Builder.CreateMemCpy(Loc,
+ createUnnamedGlobalForMemcpyFrom(
+ CGM, D, Builder, constant,
[PATCH] D63157: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue conversion applied to a member access or similar not-quite-trivial lvalue expression.
rjmccall added a comment. Minor comment request, then LGTM. Comment at: lib/CodeGen/CGDecl.cpp:1102 + // Form a simple per-variable LRU cache of these values in case we find we + // want to reuse them. + llvm::GlobalVariable * = InitializerConstants[]; I don't think this cache is LRU in any meaningful sense; it just grows unbounded. There's this thing about requiring the initializer to match, but that's not LRU. Comment at: lib/CodeGen/CGExpr.cpp:1429 +/// for instance if a block or lambda or a member of a local class uses a +/// const int variable or constexpr variable from an enclosing function. CodeGenFunction::ConstantEmission rsmith wrote: > rjmccall wrote: > > Isn't the old comment correct here? This is mandatory because of the > > enclosing-local-scope issues; that might be an "optimization" in the > > language, but it's not an optimization at the IRGen level because Sema > > literally is forcing us to do it. > In the absence of this code, we'd emit the variable as a global constant from > `EmitDeclRefLValue` in the enclosing-local-scope case (as we now do in the > more-complex cases), so this should never be necessary for correct code > generation any more. Hmm, alright. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63157/new/ https://reviews.llvm.org/D63157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D63157: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue conversion applied to a member access or similar not-quite-trivial lvalue expression.
rsmith added inline comments.
Comment at: lib/CodeGen/CGExpr.cpp:1429
+/// for instance if a block or lambda or a member of a local class uses a
+/// const int variable or constexpr variable from an enclosing function.
CodeGenFunction::ConstantEmission
rjmccall wrote:
> Isn't the old comment correct here? This is mandatory because of the
> enclosing-local-scope issues; that might be an "optimization" in the
> language, but it's not an optimization at the IRGen level because Sema
> literally is forcing us to do it.
In the absence of this code, we'd emit the variable as a global constant from
`EmitDeclRefLValue` in the enclosing-local-scope case (as we now do in the
more-complex cases), so this should never be necessary for correct code
generation any more.
Comment at: lib/Sema/SemaExpr.cpp:15808
+namespace {
+// Helper to copy the template arguments from a DeclRefExpr or MemberExpr.
+class CopiedTemplateArgs {
rjmccall wrote:
> This is really cute; I might steal this idea. That said, it's probably worth
> explaining the lifetime mechanics here so that non-experts can understand how
> this works.
Done. I also added a `[[clang::lifetimebound]]` attribute so we'll get a
warning if this is misused.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63157/new/
https://reviews.llvm.org/D63157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D63157: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue conversion applied to a member access or similar not-quite-trivial lvalue expression.
rsmith updated this revision to Diff 204589.
rsmith marked 2 inline comments as done.
rsmith added a comment.
Updated comment, fixed typo, added use of `[[clang::lifetimebound]]`.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63157/new/
https://reviews.llvm.org/D63157
Files:
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CodeGenModule.h
lib/Sema/SemaExpr.cpp
test/CXX/basic/basic.def.odr/p2.cpp
test/CXX/drs/dr20xx.cpp
test/CXX/drs/dr21xx.cpp
test/CXX/drs/dr23xx.cpp
test/CXX/drs/dr6xx.cpp
test/CXX/drs/dr7xx.cpp
test/CodeGenCXX/no-odr-use.cpp
www/cxx_dr_status.html
Index: www/cxx_dr_status.html
===
--- www/cxx_dr_status.html
+++ www/cxx_dr_status.html
@@ -4219,7 +4219,7 @@
http://wg21.link/cwg696;>696
C++11
Use of block-scope constants in local classes
-Unknown
+Yes
http://wg21.link/cwg697;>697
@@ -4315,7 +4315,7 @@
http://wg21.link/cwg712;>712
CD3
Are integer constant operands of a conditional-expression used?
-Unknown
+Partial
http://wg21.link/cwg713;>713
@@ -12313,7 +12313,7 @@
http://wg21.link/cwg2083;>2083
DR
Incorrect cases of odr-use
-Unknown
+Partial
http://wg21.link/cwg2084;>2084
@@ -12433,7 +12433,7 @@
http://wg21.link/cwg2103;>2103
DR
Lvalue-to-rvalue conversion is irrelevant in odr-use of a reference
-Unknown
+Yes
http://wg21.link/cwg2104;>2104
@@ -12835,7 +12835,7 @@
http://wg21.link/cwg2170;>2170
DR
Unclear definition of odr-use for arrays
-Unknown
+SVN
http://wg21.link/cwg2171;>2171
@@ -13933,7 +13933,7 @@
http://wg21.link/cwg2353;>2353
DR
Potential results of a member access expression for a static data member
-Unknown
+SVN
http://wg21.link/cwg2354;>2354
Index: test/CodeGenCXX/no-odr-use.cpp
===
--- /dev/null
+++ test/CodeGenCXX/no-odr-use.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -emit-llvm -o - -triple x86_64-linux-gnu %s | FileCheck %s
+
+// CHECK: @__const._Z1fi.a = private unnamed_addr constant {{.*}} { i32 1, [2 x i32] [i32 2, i32 3], [3 x i32] [i32 4, i32 5, i32 6] }
+
+struct A { int x, y[2]; int arr[3]; };
+// CHECK-LABEL: define i32 @_Z1fi(
+int f(int i) {
+ // CHECK: call void {{.*}}memcpy{{.*}}({{.*}}, {{.*}} @__const._Z1fi.a
+ constexpr A a = {1, 2, 3, 4, 5, 6};
+
+ // CHECK-LABEL: define {{.*}}@"_ZZ1fiENK3$_0clEiM1Ai"(
+ return [] (int n, int A::*p) {
+// CHECK: br i1
+return (n >= 0
+ // CHECK: getelementptr inbounds [3 x i32], [3 x i32]* getelementptr inbounds ({{.*}} @__const._Z1fi.a, i32 0, i32 2), i64 0, i64 %
+ ? a.arr[n]
+ // CHECK: br i1
+ : (n == -1
+// CHECK: getelementptr inbounds i8, i8* bitcast ({{.*}} @__const._Z1fi.a to i8*), i64 %
+// CHECK: bitcast i8* %{{.*}} to i32*
+// CHECK: load i32
+? a.*p
+// CHECK: getelementptr inbounds [2 x i32], [2 x i32]* getelementptr inbounds ({{.*}} @__const._Z1fi.a, i32 0, i32 1), i64 0, i64 %
+// CHECK: load i32
+: a.y[2 - n]));
+ }(i, ::x);
+}
Index: test/CXX/drs/dr7xx.cpp
===
--- test/CXX/drs/dr7xx.cpp
+++ test/CXX/drs/dr7xx.cpp
@@ -17,6 +17,42 @@
}
}
+namespace dr712 { // dr712: partial
+ void use(int);
+ void f() {
+const int a = 0; // expected-note 5{{here}}
+struct X {
+ void g(bool cond) {
+use(a);
+use((a));
+use(cond ? a : a);
+use((cond, a)); // expected-warning 2{{unused}} FIXME: should only warn once
+
+(void)a; // FIXME: expected-error {{declared in enclosing}}
+(void)(a); // FIXME: expected-error {{declared in enclosing}}
+(void)(cond ? a : a); // FIXME: expected-error 2{{declared in enclosing}}
+(void)(cond, a); // FIXME: expected-error {{declared in enclosing}} expected-warning {{unused}}
+ }
+};
+ }
+
+#if __cplusplus >= 201103L
+ void g() {
+struct A { int n; };
+constexpr A a = {0}; // expected-note 2{{here}}
+struct X {
+ void g(bool cond) {
+use(a.n);
+use(a.*::n);
+
+(void)a.n; // FIXME: expected-error {{declared in enclosing}}
+(void)(a.*::n); // FIXME: expected-error {{declared in enclosing}}
+ }
+};
+ }
+#endif
+}
+
namespace dr727 { // dr727: partial
struct A {
template struct C; // expected-note 6{{here}}
Index: test/CXX/drs/dr6xx.cpp
===
--- test/CXX/drs/dr6xx.cpp
+++ test/CXX/drs/dr6xx.cpp
@@ -1132,3 +1132,20 @@
template void f(int*); // expected-error {{ambiguous}}
}
}
+
+namespace dr696 { // dr696: yes
+ void f(const int*);
+ void g() {
+const int N = 10; // expected-note 1+{{here}}
+struct A {
+
[PATCH] D63157: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue conversion applied to a member access or similar not-quite-trivial lvalue expression.
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExpr.cpp:1429
+/// for instance if a block or lambda or a member of a local class uses a
+/// const int variable or constexpr variable from an enclosing function.
CodeGenFunction::ConstantEmission
Isn't the old comment correct here? This is mandatory because of the
enclosing-local-scope issues; that might be an "optimization" in the language,
but it's not an optimization at the IRGen level because Sema literally is
forcing us to do it.
Comment at: lib/Sema/SemaExpr.cpp:15808
+namespace {
+// Helper to copy the template arguments from a DeclRefExpr or MemberExpr.
+class CopiedTemplateArgs {
This is really cute; I might steal this idea. That said, it's probably worth
explaining the lifetime mechanics here so that non-experts can understand how
this works.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63157/new/
https://reviews.llvm.org/D63157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D63157: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue conversion applied to a member access or similar not-quite-trivial lvalue expression.
rsmith created this revision.
rsmith added a reviewer: rjmccall.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.
When a variable is named in a context where we can't directly emit a
reference to it (because we don't know for sure that it's going to be
defined, or it's from an enclosing function and not captured, or the
reference might not "work" for some reason), we emit a copy of the
variable as a global and use that for the known-to-be-read-only access.
Repository:
rC Clang
https://reviews.llvm.org/D63157
Files:
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CodeGenModule.h
lib/Sema/SemaExpr.cpp
test/CXX/basic/basic.def.odr/p2.cpp
test/CXX/drs/dr20xx.cpp
test/CXX/drs/dr21xx.cpp
test/CXX/drs/dr23xx.cpp
test/CXX/drs/dr6xx.cpp
test/CXX/drs/dr7xx.cpp
test/CodeGenCXX/no-odr-use.cpp
www/cxx_dr_status.html
Index: www/cxx_dr_status.html
===
--- www/cxx_dr_status.html
+++ www/cxx_dr_status.html
@@ -4219,7 +4219,7 @@
http://wg21.link/cwg696;>696
C++11
Use of block-scope constants in local classes
-Unknown
+Yes
http://wg21.link/cwg697;>697
@@ -4315,7 +4315,7 @@
http://wg21.link/cwg712;>712
CD3
Are integer constant operands of a conditional-expression used?
-Unknown
+Partial
http://wg21.link/cwg713;>713
@@ -12313,7 +12313,7 @@
http://wg21.link/cwg2083;>2083
DR
Incorrect cases of odr-use
-Unknown
+Partial
http://wg21.link/cwg2084;>2084
@@ -12433,7 +12433,7 @@
http://wg21.link/cwg2103;>2103
DR
Lvalue-to-rvalue conversion is irrelevant in odr-use of a reference
-Unknown
+Yes
http://wg21.link/cwg2104;>2104
@@ -12835,7 +12835,7 @@
http://wg21.link/cwg2170;>2170
DR
Unclear definition of odr-use for arrays
-Unknown
+SVN
http://wg21.link/cwg2171;>2171
@@ -13933,7 +13933,7 @@
http://wg21.link/cwg2353;>2353
DR
Potential results of a member access expression for a static data member
-Unknown
+SVN
http://wg21.link/cwg2354;>2354
Index: test/CodeGenCXX/no-odr-use.cpp
===
--- /dev/null
+++ test/CodeGenCXX/no-odr-use.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -emit-llvm -o - -triple x86_64-linux-gnu %s | FileCheck %s
+
+// CHECK: @__const._Z1fi.a = private unnamed_addr constant {{.*}} { i32 1, [2 x i32] [i32 2, i32 3], [3 x i32] [i32 4, i32 5, i32 6] }
+
+struct A { int x, y[2]; int arr[3]; };
+// CHECK-LABEL: define i32 @_Z1fi(
+int f(int i) {
+ // CHECK: call void {{.*}}memcpy{{.*}}({{.*}}, {{.*}} @__const._Z1fi.a
+ constexpr A a = {1, 2, 3, 4, 5, 6};
+
+ // CHECK-LABEL: define {{.*}}@"_ZZ1fiENK3$_0clEiM1Ai"(
+ return [] (int n, int A::*p) {
+// CHECK: br i1
+return (n >= 0
+ // CHECK: getelementptr inbounds [3 x i32], [3 x i32]* getelementptr inbounds ({{.*}} @__const._Z1fi.a, i32 0, i32 2), i64 0, i64 %
+ ? a.arr[n]
+ // CHECK: br i1
+ : (n == -1
+// CHECK: getelementptr inbounds i8, i8* bitcast ({{.*}} @__const._Z1fi.a to i8*), i64 %
+// CHECK: bitcast i8* %{{.*}} to i32*
+// CHECK: load i32
+? a.*p
+// CHECK: getelementptr inbounds [2 x i32], [2 x i32]* getelementptr inbounds ({{.*}} @__const._Z1fi.a, i32 0, i32 1), i64 0, i64 %
+// CHECK: load i32
+: a.y[2 - n]));
+ }(i, ::x);
+}
Index: test/CXX/drs/dr7xx.cpp
===
--- test/CXX/drs/dr7xx.cpp
+++ test/CXX/drs/dr7xx.cpp
@@ -17,6 +17,42 @@
}
}
+namespace dr712 { // dr712: partial
+ void use(int);
+ void f() {
+const int a = 0; // expected-note 5{{here}}
+struct X {
+ void g(bool cond) {
+use(a);
+use((a));
+use(cond ? a : a);
+use((cond, a)); // expected-warning 2{{unused}} FIXME: should only warn once
+
+(void)a; // FIXME: expected-error {{declared in enclosing}}
+(void)(a); // FIXME: expected-error {{declared in enclosing}}
+(void)(cond ? a : a); // FIXME: expected-error 2{{declared in enclosing}}
+(void)(cond, a); // FIXME: expected-error {{declared in enclosing}} expected-warning {{unused}}
+ }
+};
+ }
+
+#if __cplusplus >= 201103L
+ void g() {
+struct A { int n; };
+constexpr A a = {0}; // expected-note 2{{here}}
+struct X {
+ void g(bool cond) {
+use(a.n);
+use(a.*::n);
+
+(void)a.n; // FIXME: expected-error {{declared in enclosing}}
+(void)(a.*::n); // FIXME: expected-error {{declared in enclosing}}
+ }
+};
+ }
+#endif
+}
+
namespace dr727 { // dr727: partial
struct A {
template struct C; // expected-note 6{{here}}
Index: test/CXX/drs/dr6xx.cpp
===
--- test/CXX/drs/dr6xx.cpp
+++
