[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-05-09 Thread Simeon Ehrig via Phabricator via cfe-commits
SimeonEhrig added a comment. Okay, I will close the request and thank you very much for your help and your hints. https://reviews.llvm.org/D44435 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-05-08 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. Great! Let's close this review then. And good luck with cling. https://reviews.llvm.org/D44435 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-05-07 Thread Simeon Ehrig via Phabricator via cfe-commits
SimeonEhrig added a comment. In https://reviews.llvm.org/D44435#1088019, @tra wrote: > Perhaps we should take a step back and consider whether this is the right > approach to solve your problem. > > If I understand it correctly, the real issue is that you repeatedly recompile > the same module

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-05-04 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. Perhaps we should take a step back and consider whether this is the right approach to solve your problem. If I understand it correctly, the real issue is that you repeatedly recompile the same module and cling will only use the function from the first module it's seen it i

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-04-25 Thread Simeon Ehrig via Phabricator via cfe-commits
SimeonEhrig added inline comments. Comment at: lib/CodeGen/CGCUDANV.cpp:287 +CtorSuffix.append("_"); +CtorSuffix.append(ModuleName); + } tra wrote: > SimeonEhrig wrote: > > tra wrote: > > > There is a general problem with this approach. File name can con

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-04-24 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments. Comment at: lib/CodeGen/CGCUDANV.cpp:287 +CtorSuffix.append("_"); +CtorSuffix.append(ModuleName); + } SimeonEhrig wrote: > tra wrote: > > There is a general problem with this approach. File name can contain the > > characters

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-04-24 Thread Simeon Ehrig via Phabricator via cfe-commits
SimeonEhrig updated this revision to Diff 143706. SimeonEhrig added a comment. Add a comment, which declares the need of a unique ctor/dotr name. https://reviews.llvm.org/D44435 Files: lib/CodeGen/CGCUDANV.cpp unittests/CodeGen/IncrementalProcessingTest.cpp Index: unittests/CodeGen/Increme

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-04-21 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. This does not address my review. My review is suggesting that we avoid this issue completely by fixing IRGen to use an external linkage for internal declarations in your emission mode. That would allow you to just emit the module ctors as truly internal in the first

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-04-20 Thread Simeon Ehrig via Phabricator via cfe-commits
SimeonEhrig added inline comments. Comment at: lib/CodeGen/CGCUDANV.cpp:287 +CtorSuffix.append("_"); +CtorSuffix.append(ModuleName); + } tra wrote: > There is a general problem with this approach. File name can contain the > characters that PTX does not

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-04-20 Thread Simeon Ehrig via Phabricator via cfe-commits
SimeonEhrig updated this revision to Diff 143246. SimeonEhrig added a comment. Add full context with -U99 to diff. https://reviews.llvm.org/D44435 Files: lib/CodeGen/CGCUDANV.cpp unittests/CodeGen/IncrementalProcessingTest.cpp Index: unittests/CodeGen/IncrementalProcessingTest.cpp

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-04-19 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments. Comment at: lib/CodeGen/CGCUDANV.cpp:287 +CtorSuffix.append("_"); +CtorSuffix.append(ModuleName); + } There is a general problem with this approach. File name can contain the characters that PTX does not allow. We currently on

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-04-19 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment. Could you please resubmit your patch with complete context? https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface https://reviews.llvm.org/D44435 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-04-18 Thread Simeon Ehrig via Phabricator via cfe-commits
SimeonEhrig marked 2 inline comments as done. SimeonEhrig added inline comments. Comment at: lib/CodeGen/CGCUDANV.cpp:358 + if (ModuleName.empty()) +ModuleName = ""; + rjmccall wrote: > This doesn't actually seem more useful than the empty string. We improve

[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name

2018-04-18 Thread Simeon Ehrig via Phabricator via cfe-commits
SimeonEhrig updated this revision to Diff 142921. SimeonEhrig added a comment. Thank you everyone for your review comments! We addressed the inline comments and improved the description of the change set for clarity and context. Tests are updated as well. This now implements the same fix as pre