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 in. Unlike regular functions that presumably remain the same in all the
modules they are present in, CUDA constructors do change and you need cling to
grab the one from the most recent module.
This patch deals with the issue by attempting to add a unique sufix. Presumably
cling will then generate some sort of unique module name and will get unique
constructor name in return. The down side of this approach is that module name
is something that is derived from the file name and the functionality you're
changing is in the shared code, so you need to make sure that whatever you
implement makes sense for LLVM in general and that it does what it claims it
does. AFAICT, LLVM has no pressing need for the unique constructor name -- it's
a function with internal linkage and, if we ever need to generate more than
one, LLVM is capable of generating unique names within the module all by
itself. The patch currently does not fulfill the "unique" part either.
Perhaps you should consider a different approach which could handle the issue
completely in cling. E.g. You could rename the constructor in the module's IR
before passing it to JIT. Or you could rename it in PTX (it's just text after
all) before passing it to driver or PTXAS.
================
Comment at: lib/CodeGen/CGCUDANV.cpp:287
+ CtorSuffix.append("_");
+ CtorSuffix.append(ModuleName);
+ }
----------------
SimeonEhrig wrote:
> tra wrote:
> > SimeonEhrig wrote:
> > > tra wrote:
> > > > There is a general problem with this approach. File name can contain
> > > > the characters that PTX does not allow.
> > > > We currently only deal with '.' and '@', but that's not enough here.
> > > > You may want to either mangle the name somehow to avoid/convert illegal
> > > > characters or use some other way to provide unique suffix. Hex-encoded
> > > > hash of the file name would avoid this problem, for example.
> > > >
> > > >
> > > >
> > > Maybe I'm wrong but I think, that should be no problem, because the
> > > generating of a cuda ctor/dtor have nothing to do with the PTX
> > > generation.
> > >
> > > The function 'makeModuleCtorFunction' should just generate llvm ir code
> > > for the host (e.g. x86_64).
> > >
> > > If I'm wrong, could you tell me please, where in the source code the
> > > 'makeModuleCtorFunction' affect the PTX generation.
> > You are correct that PTX is irrelevant here. I've completely missed that
> > this will be generated for the host, which is more forgiving.
> >
> > That said, I'm still not completely sure whether we're guaranteed that
> > using arbitrary characters in a symbol name is OK on x86 and, potentially,
> > other host platforms. As an experiment, try using a module which has a
> > space in its name.
> At line 295 and 380 in CGCUDANV.cpp I use a sanitizer function, which replace
> all symbols without [a-zA-Z0-9._] with a '_'. It's the same solution like in
> D34059. So I think, it would works in general.
>
> Only for information. I tested it with a module name, which includes a
> whitespace and without the sanitizer. It works on Linux x86 and the ELF
> format. There was an whitespace in the symbol of the cuda module ctor (I
> checked it with readelf).
>
> In general, do you think my solution approach is technically okay? Your
> answer will be really helpful for internal usage in our cling project. At the
> moment I developed the cling-cuda-interpreter based on this patch and it
> would helps a lot of, if I can say, that the patch doesn't cause any problem
> with the CUDA-environment.
This still does not give you unique suffix which was stated at the main goal of
this patch. E.g files "foo$bar" and "foo_bar" will produce identical names. See
my previous comment regarding using a hash.
https://reviews.llvm.org/D44435
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
