mehdi_amini added a comment.
> I'd like to also see a testcase for the situation where we trigger the
> emission of a declaration with an available_externally definition and then
> find we need to promote it to a "full" definition:
Added!
Comment at: clang/lib/CodeGen/CodeGe
This revision was automatically updated to reflect the committed changes.
Closed by commit rL311857: Emit static constexpr member as available_externally
definition (authored by mehdi_amini).
Changed prior to commit:
https://reviews.llvm.org/D34992?vs=109694&id=112836#toc
Repository:
rL LLVM
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
I'd like to also see a testcase for the situation where we trigger the emission
of a declaration with an `available_externally` definition and then find we
need to promote it to a "full" defin
arphaman added a comment.
FYI, this doesn't compiler after John's constant emitter refactoring (r310964)
https://reviews.llvm.org/D34992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
mehdi_amini marked an inline comment as done.
mehdi_amini added a comment.
Bi-weekly ping! (@rsmith)
https://reviews.llvm.org/D34992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mehdi_amini marked 6 inline comments as done.
mehdi_amini added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2293
/// If ExcludeCtor is true, the duration when the object's constructor runs
-/// will not be considered. The caller will need to verify that the
mehdi_amini updated this revision to Diff 109694.
mehdi_amini marked 12 inline comments as done.
mehdi_amini added a comment.
Address Richard's comment
https://reviews.llvm.org/D34992
Files:
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp
Index: clang/
rsmith added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2293
/// If ExcludeCtor is true, the duration when the object's constructor runs
-/// will not be considered. The caller will need to verify that the object is
-/// not written to during its constructi
mehdi_amini added a comment.
Ping again @rsmith (or anyone else)
https://reviews.llvm.org/D34992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mehdi_amini added a comment.
Weekly ping! (@rsmith)
https://reviews.llvm.org/D34992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mehdi_amini added a comment.
@rsmith: post-C++-commitee-meeting ping ;)
https://reviews.llvm.org/D34992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mehdi_amini updated this revision to Diff 105787.
mehdi_amini added a comment.
Fix issues around mutable fields and regression on "internal", add more testing.
https://reviews.llvm.org/D34992
Files:
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp
Index:
rsmith added inline comments.
Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:15
+// CHECK: @_ZL4BAR3 = available_externally constant i32 44,
+static constexpr int BAR3 = 44;
+
mehdi_amini wrote:
> rsmith wrote:
> > mehdi_amini wrote:
> > > Looks lik
mehdi_amini added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2374
+ llvm::Constant *Init = nullptr;
+ if (D && D->isConstexpr() && !D->isInline() && !D->hasAttr())
{
+const VarDecl *InitDecl;
rsmith wrote:
> Applying this for only `co
rsmith added a comment.
We've had problems in the past with speculative emission of values like this
resulting in us producing invalid symbol references. (Two cases I remember: an
`available_externally` symbol references a symbol that should be emitted as
`linkonce_odr` but which is not emitted
mehdi_amini added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2376
+const VarDecl *InitDecl;
+const Expr *InitExpr = D->getAnyInitializer(InitDecl);
+if (InitExpr) {
ahatanak wrote:
> Does getAnyInitializer ever return a null poin
ahatanak added inline comments.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2376
+const VarDecl *InitDecl;
+const Expr *InitExpr = D->getAnyInitializer(InitDecl);
+if (InitExpr) {
Does getAnyInitializer ever return a null pointer here when D is a
mehdi_amini added inline comments.
Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:12
+// CHECK: @_ZN3Foo4BAR2E = external constant i32,
+ static const int BAR2 = 43;
+};
Note: I'm not sure if the standard allows us to assume that we can fold the
On Wed, Jul 5, 2017 at 12:22 PM Mehdi AMINI wrote:
> The LLVM verifier is complaining that dllimport have to be external
> linkage and isn't happy with available_externally, is the verifier wrong?
>
IMO, yes. I imagine that it is fine with dllimport available_externally
functions already.
> 20
The LLVM verifier is complaining that dllimport have to be external linkage
and isn't happy with available_externally, is the verifier wrong?
2017-07-05 9:12 GMT-07:00 David Majnemer :
> I don't think you need the dllimport restriction.
>
> On Wed, Jul 5, 2017 at 12:05 PM Alex Lorenz via Phabrica
I don't think you need the dllimport restriction.
On Wed, Jul 5, 2017 at 12:05 PM Alex Lorenz via Phabricator via cfe-commits
wrote:
> arphaman added a comment.
>
> Does this apply to all constexpr global variables? It could potentially
> fix https://bugs.llvm.org/show_bug.cgi?id=31860 .
>
>
> h
arphaman added a comment.
Does this apply to all constexpr global variables? It could potentially fix
https://bugs.llvm.org/show_bug.cgi?id=31860 .
https://reviews.llvm.org/D34992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists
mehdi_amini created this revision.
This enables better optimization, I don't if it is legal c++11 though.
https://reviews.llvm.org/D34992
Files:
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp
Index: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp
===
23 matches
Mail list logo
