jdoerfert added subscribers: xtian, gregrodgers, ddibyend.
jdoerfert added a comment.
Herald added a subscriber: ormris.
Could you sketch for me how this will (potentially) work if we have multiple
target vendors? The fatbin solution seems tailored to NVIDIA, but maybe I'm
wrong here.
In any ca
gtbercea added a comment.
ping
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D47394/new/
https://reviews.llvm.org/D47394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/l
gtbercea updated this revision to Diff 187979.
gtbercea added a comment.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.
- Update.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D47394/new/
https://reviews.llvm.org/D47394
Files:
include/clan
gtbercea updated this revision to Diff 159536.
gtbercea marked 3 inline comments as done.
gtbercea added a comment.
- Address comments.
Repository:
rC Clang
https://reviews.llvm.org/D47394
Files:
include/clang/Driver/Action.h
include/clang/Driver/Compilation.h
include/clang/Driver/Opti
gtbercea added inline comments.
Comment at: lib/Driver/ToolChains/Cuda.cpp:664
+ // Anything that's not a file name is potentially a static library
+ // so treat it as such.
+ if (C.canSkipOffloadBundler())
sfantao wrote:
> So, what if it is not a
gtbercea added inline comments.
Comment at: include/clang/Driver/Compilation.h:312
+ /// \param skipBundler - bool value set once by the driver.
+ void setSkipOffloadBundler(bool skipBundler);
+
sfantao wrote:
> gtbercea wrote:
> > sfantao wrote:
> > > gtbercea
sfantao added inline comments.
Comment at: include/clang/Driver/Compilation.h:312
+ /// \param skipBundler - bool value set once by the driver.
+ void setSkipOffloadBundler(bool skipBundler);
+
gtbercea wrote:
> sfantao wrote:
> > gtbercea wrote:
> > > sfantao
gtbercea marked 3 inline comments as done.
gtbercea added a comment.
Answers to comments.
Comment at: include/clang/Driver/Compilation.h:312
+ /// \param skipBundler - bool value set once by the driver.
+ void setSkipOffloadBundler(bool skipBundler);
+
sfanta
sfantao added a comment.
Hi Doru,
Thanks for updating the patch. I've a few comments below.
Comment at: include/clang/Driver/Compilation.h:312
+ /// \param skipBundler - bool value set once by the driver.
+ void setSkipOffloadBundler(bool skipBundler);
+
gtb
gtbercea updated this revision to Diff 150947.
gtbercea added a comment.
Added separate test.
Repository:
rC Clang
https://reviews.llvm.org/D47394
Files:
include/clang/Driver/Action.h
include/clang/Driver/Compilation.h
include/clang/Driver/Driver.h
include/clang/Driver/Options.td
i
gtbercea marked 3 inline comments as done.
gtbercea added inline comments.
Comment at: include/clang/Driver/Compilation.h:312
+ /// \param skipBundler - bool value set once by the driver.
+ void setSkipOffloadBundler(bool skipBundler);
+
sfantao wrote:
> Why is
gtbercea added a comment.
@tra Thank you for your comments and help with the patch.
Repository:
rC Clang
https://reviews.llvm.org/D47394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co
Hahnfeld added a comment.
In https://reviews.llvm.org/D47394#1123044, @tra wrote:
> While I'm not completely convinced that [fatbin]->.c->[clang]->.o (with
> device code only)->[ld -r] -> host.o (host+device code) is ideal (things
> could be done with smaller number of tool invocations), it sho
tra added a comment.
With the updated patch description + the discussion I'm OK with the approach
from the general "how do we compile/use CUDA" point of view. I'll leave the
question of whether the approach works for OpenMP to someone more familiar with
it.
While I'm not completely convinced t
gtbercea added a comment.
In https://reviews.llvm.org/D47394#1120255, @Hahnfeld wrote:
> In https://reviews.llvm.org/D47394#1119489, @gtbercea wrote:
>
> > In https://reviews.llvm.org/D47394#1119056, @Hahnfeld wrote:
> >
> > > Hmm, maybe the scope is much larger: I just tried linking an executabl
Hahnfeld added a comment.
In https://reviews.llvm.org/D47394#1119489, @gtbercea wrote:
> In https://reviews.llvm.org/D47394#1119056, @Hahnfeld wrote:
>
> > Hmm, maybe the scope is much larger: I just tried linking an executable
> > that references a `declare target` function in a shared library.
gtbercea added a comment.
In https://reviews.llvm.org/D47394#1119056, @Hahnfeld wrote:
> Hmm, maybe the scope is much larger: I just tried linking an executable that
> references a `declare target` function in a shared library. My assumption was
> that this already works, given that `libomptarg
Hahnfeld added a comment.
Hmm, maybe the scope is much larger: I just tried linking an executable that
references a `declare target` function in a shared library. My assumption was
that this already works, given that `libomptarget`'s registration functions can
be called multiple times. Am I doi
Hahnfeld added a comment.
In https://reviews.llvm.org/D47394#1118957, @gtbercea wrote:
> I'm surprised you now disagree with this technique, when I first introduced
> you to this in an e-mail off list you agreed with it.
My words were `I agree this is the best solution for NVPTX.` In the sam
gtbercea added a comment.
> I disagree in this context because this patch currently means that static
> archives will only work with NVPTX and there is no clear path how to "fix"
> things for other offloading targets. I'll try to work on my proposal over the
> next few days (sorry, very busy
Hahnfeld added a comment.
In https://reviews.llvm.org/D47394#1118393, @gtbercea wrote:
> The error is related to lack of device linking, just like you explained two
> paragraphs down. This is the error I get:
>
> main.o: In function `__cuda_module_ctor':
> main.cu:(.text+0x674): undefined re
gtbercea added a comment.
The error is related to lack of device linking, just like you explained two
paragraphs down. This is the error I get:
main.o: In function `__cuda_module_ctor':
main.cu:(.text+0x674): undefined reference to
`__cudaRegisterLinkedBinary__nv_c5b75865'
You nailed the p
tra added a comment.
In https://reviews.llvm.org/D47394#1118223, @gtbercea wrote:
> I tried this example
> (https://devblogs.nvidia.com/separate-compilation-linking-cuda-device-code/).
> It worked with NVCC but not with clang++. I can produce the main.o particle.o
> and v.o objects as relocata
gtbercea added a comment.
> Assuming we do proceed with back-to-CUDA approach, one thing I'd consider
> would be using clang's -fcuda-include-gpubinary option which CUDA uses to
> include GPU code into the host object. You may be able to use it to avoid
> compiling and partially linking .fatbin
sfantao added a comment.
> In a discussion off-list I proposed adding constructor functions to all
> object files and handle them like shared libraries are already handled today
> (ie register separately and let the runtime figure out how to relocate
> symbols in different translation units). I
Hahnfeld added a comment.
In https://reviews.llvm.org/D47394#1115086, @tra wrote:
> On one hand I can see how being able to treat GPU-side binaries as any other
> host files is convenient. On the other hand, this convenience comes with the
> price of targeting only NVPTX. This seems contrary to
tra added a comment.
"Interoperability with other compilers" is probably a statement that's a bit
too strong. At best it's kind of compatible with CUDA tools and I don't think
it's feasible for other compilers. I.e. it will be useless for AMD GPUs and
whatever compiler they use.
In general it
gtbercea added a comment.
In https://reviews.llvm.org/D47394#1114848, @sfantao wrote:
> Just to clarify one thing in my last comment:
>
> When I say that we didn't aim at having clang compatible with other
> compilers, I mean the OpenMP offloading descriptors, where all the variables
> and offl
gtbercea added inline comments.
Comment at: lib/Driver/ToolChains/Cuda.cpp:536
+ }
}
sfantao wrote:
> What prevents all this from being done in the bundler? If I understand it
> correctly, if the bundler implements this wrapping all the checks for
> librari
guansong added a comment.
I am not quite familiar with Clang driver set up, I will add Greg for more
comments. But I have hacked one the latest YKT tree to support simple AMDGCN
path the same way as NVPTX. The last patch is here
https://github.com/ROCm-Developer-Tools/hcc2-clang/commit/8c1cce0d
gtbercea added inline comments.
Comment at: test/Driver/openmp-offload.c:497
// RUN: %clang -### -fopenmp=libomp -o %t.out -lsomelib -target
powerpc64le-linux
-fopenmp-targets=powerpc64le-ibm-linux-gnu,x86_64-pc-linux-gnu %t.i
-no-canonical-prefixes 2>&1 \
// RUN: | Fil
gtbercea added inline comments.
Comment at: test/Driver/openmp-offload.c:497
// RUN: %clang -### -fopenmp=libomp -o %t.out -lsomelib -target
powerpc64le-linux
-fopenmp-targets=powerpc64le-ibm-linux-gnu,x86_64-pc-linux-gnu %t.i
-no-canonical-prefixes 2>&1 \
// RUN: | Fil
sfantao added a comment.
Just to clarify one thing in my last comment:
When I say that we didn't aim at having clang compatible with other compilers,
I mean the OpenMP offloading descriptors, where all the variables and
offloading entry points are. Of course we want to allow the resulting binar
sfantao added a comment.
@gtbercea, thanks for the patch.
> INTEROPERABILITY WITH OTHER COMPILERS: These bundled object files can end up
> being passed between Clang and other compilers which may lead to
> incompatibilities: passing a bundled file from Clang to another compiler
> would lead to
gtbercea updated this revision to Diff 148677.
Repository:
rC Clang
https://reviews.llvm.org/D47394
Files:
include/clang/Driver/Action.h
include/clang/Driver/Compilation.h
include/clang/Driver/Driver.h
include/clang/Driver/ToolChain.h
lib/Driver/Action.cpp
lib/Driver/Compilation.cp
gtbercea created this revision.
gtbercea added reviewers: Hahnfeld, hfinkel, caomhin, carlo.bertolli, tra.
Herald added subscribers: cfe-commits, guansong.
So far, the clang-offload-bundler has been the default tool for bundling
together various files types produced by the different OpenMP offloa
36 matches
Mail list logo
