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
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
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
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
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
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
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
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
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
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
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
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
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
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
14 matches
Mail list logo