sfantao abandoned this revision.
sfantao marked 8 inline comments as done.
sfantao added a comment.
Hi Jonas,
In https://reviews.llvm.org/D9888#581809, @Hahnfeld wrote:
> I think these changes have been contributed to trunk in multiple commits so
> this can be closed?
You're right, this can b
Hahnfeld added a comment.
I think these changes have been contributed to trunk in multiple commits so
this can be closed?
https://reviews.llvm.org/D9888
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/l
sfantao marked 8 inline comments as done.
sfantao added a comment.
Hi Eric,
Thanks for the review!
As you are probably a aware, I started partitioning this patch following your
initial concern related with the size of this patch and the feedback I got from
http://lists.llvm.org/pipermail/cfe-d
echristo added a comment.
First I'd like to note that the code quality here is really high, most of my
comments are higher level design decisions going with the driver and the
implementation here rather than that.
One meta comment: offload appears to be something that could be used for CUDA
an
sfantao added a comment.
Hi Michael,
In http://reviews.llvm.org/D9888#380225, @mkuron wrote:
> The three smaller patches into which you divided this one appear to be
> missing some things. For example, `AddOpenMPLinkerScript` in
> //lib/Driver/Tools.cpp// from this patch appears to still be ne
mkuron added a comment.
The three smaller patches into which you divided this one appear to be missing
some things. For example, `AddOpenMPLinkerScript` in //lib/Driver/Tools.cpp//
from this patch appears to still be necessary to get the desired functionality,
but it is not present in any of th
sfantao added a comment.
Hi Richard,
Thanks for your review. I partitioned some of the stuff I am proposing here in
smaller patches:
http://reviews.llvm.org/D18170
http://reviews.llvm.org/D18171
http://reviews.llvm.org/D18172
These patches already try to incorporate the feedback I got in
http
rsmith added a comment.
@echristo, you asked for time to review this; if you still want to, please can
you do so?
@tra, it looks like you're happy with this design (and with moving the CUDA
offloading support in this direction), please let us know if not!
Comment at: include/c
Hahnfeld added a comment.
@rsmith could you possibly take a look at this one? It has been around for
roughly 8 months now and hasn't received much feedback
http://reviews.llvm.org/D9888
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http:/
Hahnfeld added a comment.
Will this somewhen receive a final review and get merged?
http://reviews.llvm.org/D9888
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
sfantao added a comment.
Any more comments on this patch?
Thanks,
Samuel
http://reviews.llvm.org/D9888
___
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 41001.
sfantao added a comment.
Rebase.
http://reviews.llvm.org/D9888
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Driver/Action.h
include/clang/Driver/CC1Options.td
include/clang/Driver/Driver.h
include/clang/Driver/Options.t
sfantao updated this revision to Diff 39594.
sfantao added a comment.
Rebase.
http://reviews.llvm.org/D9888
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Driver/Action.h
include/clang/Driver/CC1Options.td
include/clang/Driver/Driver.h
include/clang/Driver/Options.t
sfantao updated this revision to Diff 37903.
sfantao added a comment.
Move clang-offload-bundler to to a separate review:
http://reviews.llvm.org/D13909.
This patch depends on http://reviews.llvm.org/D13909.
http://reviews.llvm.org/D9888
Files:
include/clang/Basic/DiagnosticDriverKinds.td
sfantao added a comment.
Are there any more comments/suggestions about this patch?
Thanks!
Samuel
http://reviews.llvm.org/D9888
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Hahnfeld added a comment.
In http://reviews.llvm.org/D9888#262389, @sfantao wrote:
> [...]
>
> I assume you were trying this using the diff in
> http://reviews.llvm.org/D12614. There was an inconsistency in the names of
> the ELF sections and symbols defined by the linker script in these two
>
sfantao added a comment.
In http://reviews.llvm.org/D9888#262325, @tra wrote:
> In http://reviews.llvm.org/D9888#257904, @sfantao wrote:
>
> > This diff refactors the original patch and is rebased on top of the latests
> > offloading changes inserted for CUDA.
> >
> > Here I don't touch the CUDA
sfantao added a comment.
Art, Jonas,
Thanks for the comments!
In http://reviews.llvm.org/D9888#261434, @Hahnfeld wrote:
> Currently trying to test, but
>
> 1. Offloading to the same target isn't supported (`x86_64-unknown-linux-gnu`
> as host and device) - this was working with `clang-omp` The
sfantao updated this revision to Diff 36816.
sfantao added a comment.
Make the offloading ELF sections consistent with what is in
http://reviews.llvm.org/D12614.
Fix bug in AtTopLevel flag, so that the bundling job is considered always top
level job.
Fix several typos.
http://reviews.llvm.or
tra added a comment.
In http://reviews.llvm.org/D9888#257904, @sfantao wrote:
> This diff refactors the original patch and is rebased on top of the latests
> offloading changes inserted for CUDA.
>
> Here I don't touch the CUDA support. I tried, however, to have the
> implementation modular eno
Hahnfeld added a comment.
Currently trying to test, but
1. Offloading to the same target isn't supported (`x86_64-unknown-linux-gnu` as
host and device) - this was working with `clang-omp`
The produced IR isn't showing any calls to the target library and on linkage it
complains:
undefined r
sfantao updated this revision to Diff 36263.
sfantao added a comment.
This diff refactors the original patch and is rebased on top of the latests
offloading changes inserted for CUDA.
Here I don't touch the CUDA support. I tried, however, to have the
implementation modular enough so that it cou
Hahnfeld added a comment.
I think this has to be updated for the current trunk...
http://reviews.llvm.org/D9888
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
23 matches
Mail list logo
