hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
A nice abstraction and cleanup. LGTM.
Comment at: lib/Driver/Driver.cpp:1625
@@ +1624,3 @@
+ // architecture. If we are in host-only mode we return 'success' so that
+
ABataev added a comment.
LG for me.
https://reviews.llvm.org/D18172
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
sfantao updated this revision to Diff 72117.
sfantao added a comment.
- Rebase.
https://reviews.llvm.org/D18172
Files:
include/clang/Driver/Compilation.h
lib/Driver/Driver.cpp
lib/Driver/Types.cpp
test/Driver/cuda-bindings.cu
test/Driver/cuda-phases.cu
Index:
Hahnfeld added a subscriber: Hahnfeld.
Hahnfeld added a comment.
In https://reviews.llvm.org/D18172#500029, @sfantao wrote:
> Any more comments on this patch or depending ones?
>
> Thanks!
> Samuel
I can report that the latest patches are working for me and that they fix all
points that I
sfantao added a comment.
Any more comments on this patch or depending ones?
Thanks!
Samuel
https://reviews.llvm.org/D18172
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
sfantao updated this revision to Diff 66016.
sfantao added a comment.
- Remove redundant phases from cuda-phases.cu and use DAG check.
- Rebase.
https://reviews.llvm.org/D18172
Files:
include/clang/Driver/Compilation.h
lib/Driver/Driver.cpp
test/Driver/cuda-phases.cu
Index:
sfantao updated this revision to Diff 63681.
sfantao added a comment.
- Rebase.
http://reviews.llvm.org/D18172
Files:
include/clang/Driver/Compilation.h
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
===
---
sfantao updated this revision to Diff 62573.
sfantao added a comment.
- Rebase
http://reviews.llvm.org/D18172
Files:
include/clang/Driver/Compilation.h
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
===
---
sfantao added a comment.
In http://reviews.llvm.org/D18172#472152, @jlebar wrote:
> Yeah, I'd say that in the absence of a rule, consistency with surrounding
> code is king. Otherwise we're sending a message when we don't mean to be.
>
> I'm not at my machine, but my recollection is that most
sfantao updated this revision to Diff 62506.
sfantao added a comment.
- Use double instead of triple slash in one comment.
http://reviews.llvm.org/D18172
Files:
include/clang/Driver/Compilation.h
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
jlebar added a subscriber: jlebar.
jlebar added a comment.
Yeah, I'd say that in the absence of a rule, consistency with surrounding
code is king. Otherwise we're sending a message when we don't mean to be.
I'm not at my machine, but my recollection is that most of the driver uses
final
Yeah, I'd say that in the absence of a rule, consistency with surrounding
code is king. Otherwise we're sending a message when we don't mean to be.
I'm not at my machine, but my recollection is that most of the driver uses
final sparingly. But whatever the convention is we should do that, I
sfantao added a comment.
In http://reviews.llvm.org/D18172#471861, @jlebar wrote:
> Alexey, it seems that you're asking for "final" on all classes that are not
> inherited from. Forgive my ignorance, but would you mind pointing me to the
> document that talks about our position on "final" in
ABataev added a comment.
In http://reviews.llvm.org/D18172#471861, @jlebar wrote:
> Alexey, it seems that you're asking for "final" on all classes that are not
> inherited from. Forgive my ignorance, but would you mind pointing me to the
> document that talks about our position on "final" in
jlebar added a comment.
Alexey, it seems that you're asking for "final" on all classes that are not
inherited from. Forgive my ignorance, but would you mind pointing me to the
document that talks about our position on "final" in LLVM source? I don't see
it in the style guide, but I may be
sfantao added a comment.
Hi Alexey,
Thanks for the review! Addressed your comments in the new diff.
I'll wait for your response on my question in http://reviews.llvm.org/D18172 to
address the issue with \brief properly.
Thanks again,
Samuel
http://reviews.llvm.org/D18172
sfantao updated this revision to Diff 62438.
sfantao marked 3 inline comments as done.
sfantao added a comment.
- Fix comments, annotate classes with final, and add default initializers.
http://reviews.llvm.org/D18172
Files:
include/clang/Driver/Compilation.h
lib/Driver/Driver.cpp
Index:
ABataev added a comment.
No '\brief's
Comment at: lib/Driver/Driver.cpp:1393
@@ +1392,3 @@
+/// generate the required device actions.
+class OffloadingActionBuilder {
+ /// \brief Flag used to trace errors in the builder.
1. 'final'
2. default initializers for
sfantao updated this revision to Diff 62231.
sfantao added a comment.
- Rebase.
http://reviews.llvm.org/D18172
Files:
include/clang/Driver/Compilation.h
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
===
---
sfantao updated this revision to Diff 52881.
sfantao updated the summary for this revision.
sfantao added a comment.
Rebase.
http://reviews.llvm.org/D18172
Files:
include/clang/Driver/Compilation.h
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
20 matches
Mail list logo
