[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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/cfe-commits
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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 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. You are right. The clang commit is not the best solution. So, we searched for another solution and found one. The solution is similar to your suggestion. We found a possibility to integrate a llvm module pass, which detects the symbols `__cuda_module_ctor` and `__cuda_module_dtor` and append the module name to the symbol, before the llvm IR will be generated. So, we were able to move the solution from clang to cling, which is better for both projects. https://reviews.llvm.org/D44435 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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 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.
https://reviews.llvm.org/D44435
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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 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.
https://reviews.llvm.org/D44435
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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/IncrementalProcessingTest.cpp
===
--- unittests/CodeGen/IncrementalProcessingTest.cpp
+++ unittests/CodeGen/IncrementalProcessingTest.cpp
@@ -21,9 +21,11 @@
#include "llvm/IR/Module.h"
#include "llvm/Support/Host.h"
#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Target/TargetOptions.h"
#include "gtest/gtest.h"
#include
+#include
using namespace llvm;
using namespace clang;
@@ -171,4 +173,122 @@
}
+
+// In CUDA incremental processing, a CUDA ctor or dtor will be generated for
+// every statement if a fatbinary file exists.
+const char CUDATestProgram1[] =
+"void cudaFunc1(){}\n";
+
+const char CUDATestProgram2[] =
+"void cudaFunc2(){}\n";
+
+const Function* getCUDActor(llvm::Module& M) {
+ for (const auto& Func: M)
+if (Func.hasName() && Func.getName().startswith("__cuda_module_ctor_"))
+ return
+
+ return nullptr;
+}
+
+const Function* getCUDAdtor(llvm::Module& M) {
+ for (const auto& Func: M)
+if (Func.hasName() && Func.getName().startswith("__cuda_module_dtor_"))
+ return
+
+ return nullptr;
+}
+
+TEST(IncrementalProcessing, EmitCUDAGlobalInitFunc) {
+LLVMContext Context;
+CompilerInstance compiler;
+
+compiler.createDiagnostics();
+compiler.getLangOpts().CPlusPlus = 1;
+compiler.getLangOpts().CPlusPlus11 = 1;
+compiler.getLangOpts().CUDA = 1;
+
+compiler.getTargetOpts().Triple = llvm::Triple::normalize(
+llvm::sys::getProcessTriple());
+compiler.setTarget(clang::TargetInfo::CreateTargetInfo(
+ compiler.getDiagnostics(),
+ std::make_shared(
+compiler.getTargetOpts(;
+
+// To enable the generating of cuda host code, it's needs to set up the
+// auxTriple.
+llvm::Triple hostTriple(llvm::sys::getProcessTriple());
+compiler.getFrontendOpts().AuxTriple =
+hostTriple.isArch64Bit() ? "nvptx64-nvidia-cuda" : "nvptx-nvidia-cuda";
+auto targetOptions = std::make_shared();
+targetOptions->Triple = compiler.getFrontendOpts().AuxTriple;
+targetOptions->HostTriple = compiler.getTarget().getTriple().str();
+compiler.setAuxTarget(clang::TargetInfo::CreateTargetInfo(
+compiler.getDiagnostics(), targetOptions));
+
+// A fatbinary file is necessary, that the code generator generates the ctor
+// and dtor.
+auto tmpFatbinFileOrError = llvm::sys::fs::TempFile::create("dummy.fatbin");
+ASSERT_TRUE((bool)tmpFatbinFileOrError);
+auto tmpFatbinFile = std::move(*tmpFatbinFileOrError);
+compiler.getCodeGenOpts().CudaGpuBinaryFileName = tmpFatbinFile.TmpName;
+
+compiler.createFileManager();
+compiler.createSourceManager(compiler.getFileManager());
+compiler.createPreprocessor(clang::TU_Prefix);
+compiler.getPreprocessor().enableIncrementalProcessing();
+
+compiler.createASTContext();
+
+CodeGenerator* CG =
+CreateLLVMCodeGen(
+compiler.getDiagnostics(),
+"main-module",
+compiler.getHeaderSearchOpts(),
+compiler.getPreprocessorOpts(),
+compiler.getCodeGenOpts(),
+Context);
+
+compiler.setASTConsumer(std::unique_ptr(CG));
+compiler.createSema(clang::TU_Prefix, nullptr);
+Sema& S = compiler.getSema();
+
+std::unique_ptr ParseOP(new Parser(S.getPreprocessor(), S,
+ /*SkipFunctionBodies*/ false));
+Parser = *ParseOP.get();
+
+std::array M;
+M[0] = IncrementalParseAST(compiler, P, *CG, nullptr);
+ASSERT_TRUE(M[0]);
+
+M[1] = IncrementalParseAST(compiler, P, *CG, CUDATestProgram1);
+ASSERT_TRUE(M[1]);
+ASSERT_TRUE(M[1]->getFunction("_Z9cudaFunc1v"));
+
+M[2] = IncrementalParseAST(compiler, P, *CG, CUDATestProgram2);
+ASSERT_TRUE(M[2]);
+ASSERT_TRUE(M[2]->getFunction("_Z9cudaFunc2v"));
+// First code should not end up in second module:
+ASSERT_FALSE(M[2]->getFunction("_Z9cudaFunc1v"));
+
+// Make sure, that cuda ctor's and dtor's exist:
+const Function* CUDActor1 = getCUDActor(*M[1]);
+ASSERT_TRUE(CUDActor1);
+
+const Function* CUDActor2 = getCUDActor(*M[2]);
+ASSERT_TRUE(CUDActor2);
+
+const Function* CUDAdtor1 = getCUDAdtor(*M[1]);
+ASSERT_TRUE(CUDAdtor1);
+
+const Function* CUDAdtor2 = getCUDAdtor(*M[2]);
+ASSERT_TRUE(CUDAdtor2);
+
+// Compare the names of both ctor's and dtor's to check, that they are
+// unique.
+ASSERT_FALSE(CUDActor1->getName() == CUDActor2->getName());
+ASSERT_FALSE(CUDAdtor1->getName() == CUDAdtor2->getName());
+
+ASSERT_FALSE((bool)tmpFatbinFile.discard());
+}
+
} // end anonymous
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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 place, removing any need to mangle them. https://reviews.llvm.org/D44435 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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 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.
https://reviews.llvm.org/D44435
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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
===
--- unittests/CodeGen/IncrementalProcessingTest.cpp
+++ unittests/CodeGen/IncrementalProcessingTest.cpp
@@ -21,9 +21,11 @@
#include "llvm/IR/Module.h"
#include "llvm/Support/Host.h"
#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Target/TargetOptions.h"
#include "gtest/gtest.h"
#include
+#include
using namespace llvm;
using namespace clang;
@@ -171,4 +173,122 @@
}
+
+// In CUDA incremental processing, a CUDA ctor or dtor will be generated for
+// every statement if a fatbinary file exists.
+const char CUDATestProgram1[] =
+"void cudaFunc1(){}\n";
+
+const char CUDATestProgram2[] =
+"void cudaFunc2(){}\n";
+
+const Function* getCUDActor(llvm::Module& M) {
+ for (const auto& Func: M)
+if (Func.hasName() && Func.getName().startswith("__cuda_module_ctor_"))
+ return
+
+ return nullptr;
+}
+
+const Function* getCUDAdtor(llvm::Module& M) {
+ for (const auto& Func: M)
+if (Func.hasName() && Func.getName().startswith("__cuda_module_dtor_"))
+ return
+
+ return nullptr;
+}
+
+TEST(IncrementalProcessing, EmitCUDAGlobalInitFunc) {
+LLVMContext Context;
+CompilerInstance compiler;
+
+compiler.createDiagnostics();
+compiler.getLangOpts().CPlusPlus = 1;
+compiler.getLangOpts().CPlusPlus11 = 1;
+compiler.getLangOpts().CUDA = 1;
+
+compiler.getTargetOpts().Triple = llvm::Triple::normalize(
+llvm::sys::getProcessTriple());
+compiler.setTarget(clang::TargetInfo::CreateTargetInfo(
+ compiler.getDiagnostics(),
+ std::make_shared(
+compiler.getTargetOpts(;
+
+// To enable the generating of cuda host code, it's needs to set up the
+// auxTriple.
+llvm::Triple hostTriple(llvm::sys::getProcessTriple());
+compiler.getFrontendOpts().AuxTriple =
+hostTriple.isArch64Bit() ? "nvptx64-nvidia-cuda" : "nvptx-nvidia-cuda";
+auto targetOptions = std::make_shared();
+targetOptions->Triple = compiler.getFrontendOpts().AuxTriple;
+targetOptions->HostTriple = compiler.getTarget().getTriple().str();
+compiler.setAuxTarget(clang::TargetInfo::CreateTargetInfo(
+compiler.getDiagnostics(), targetOptions));
+
+// A fatbinary file is necessary, that the code generator generates the ctor
+// and dtor.
+auto tmpFatbinFileOrError = llvm::sys::fs::TempFile::create("dummy.fatbin");
+ASSERT_TRUE((bool)tmpFatbinFileOrError);
+auto tmpFatbinFile = std::move(*tmpFatbinFileOrError);
+compiler.getCodeGenOpts().CudaGpuBinaryFileName = tmpFatbinFile.TmpName;
+
+compiler.createFileManager();
+compiler.createSourceManager(compiler.getFileManager());
+compiler.createPreprocessor(clang::TU_Prefix);
+compiler.getPreprocessor().enableIncrementalProcessing();
+
+compiler.createASTContext();
+
+CodeGenerator* CG =
+CreateLLVMCodeGen(
+compiler.getDiagnostics(),
+"main-module",
+compiler.getHeaderSearchOpts(),
+compiler.getPreprocessorOpts(),
+compiler.getCodeGenOpts(),
+Context);
+
+compiler.setASTConsumer(std::unique_ptr(CG));
+compiler.createSema(clang::TU_Prefix, nullptr);
+Sema& S = compiler.getSema();
+
+std::unique_ptr ParseOP(new Parser(S.getPreprocessor(), S,
+ /*SkipFunctionBodies*/ false));
+Parser = *ParseOP.get();
+
+std::array M;
+M[0] = IncrementalParseAST(compiler, P, *CG, nullptr);
+ASSERT_TRUE(M[0]);
+
+M[1] = IncrementalParseAST(compiler, P, *CG, CUDATestProgram1);
+ASSERT_TRUE(M[1]);
+ASSERT_TRUE(M[1]->getFunction("_Z9cudaFunc1v"));
+
+M[2] = IncrementalParseAST(compiler, P, *CG, CUDATestProgram2);
+ASSERT_TRUE(M[2]);
+ASSERT_TRUE(M[2]->getFunction("_Z9cudaFunc2v"));
+// First code should not end up in second module:
+ASSERT_FALSE(M[2]->getFunction("_Z9cudaFunc1v"));
+
+// Make sure, that cuda ctor's and dtor's exist:
+const Function* CUDActor1 = getCUDActor(*M[1]);
+ASSERT_TRUE(CUDActor1);
+
+const Function* CUDActor2 = getCUDActor(*M[2]);
+ASSERT_TRUE(CUDActor2);
+
+const Function* CUDAdtor1 = getCUDAdtor(*M[1]);
+ASSERT_TRUE(CUDAdtor1);
+
+const Function* CUDAdtor2 = getCUDAdtor(*M[2]);
+ASSERT_TRUE(CUDAdtor2);
+
+// Compare the names of both ctor's and dtor's to check, that they are
+// unique.
+ASSERT_FALSE(CUDActor1->getName() == CUDActor2->getName());
+ASSERT_FALSE(CUDAdtor1->getName() == CUDAdtor2->getName());
+
+ASSERT_FALSE((bool)tmpFatbinFile.discard());
+}
+
} // end anonymous namespace
Index:
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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 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.
https://reviews.llvm.org/D44435
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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 improved the implementation. If there is no module name, it will not append any suffix and the symbol is just '__cuda_module_ctor'. https://reviews.llvm.org/D44435 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44435: CUDA ctor/dtor Module-Unique Symbol Name
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 previously received in
https://reviews.llvm.org/D34059 but just for CUDA.
https://reviews.llvm.org/D44435
Files:
lib/CodeGen/CGCUDANV.cpp
unittests/CodeGen/IncrementalProcessingTest.cpp
Index: unittests/CodeGen/IncrementalProcessingTest.cpp
===
--- unittests/CodeGen/IncrementalProcessingTest.cpp
+++ unittests/CodeGen/IncrementalProcessingTest.cpp
@@ -21,9 +21,11 @@
#include "llvm/IR/Module.h"
#include "llvm/Support/Host.h"
#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Target/TargetOptions.h"
#include "gtest/gtest.h"
#include
+#include
using namespace llvm;
using namespace clang;
@@ -171,4 +173,122 @@
}
+
+// In CUDA incremental processing, a CUDA ctor or dtor will be generated for
+// every statement if a fatbinary file exists.
+const char CUDATestProgram1[] =
+"void cudaFunc1(){}\n";
+
+const char CUDATestProgram2[] =
+"void cudaFunc2(){}\n";
+
+const Function* getCUDActor(llvm::Module& M) {
+ for (const auto& Func: M)
+if (Func.hasName() && Func.getName().startswith("__cuda_module_ctor_"))
+ return
+
+ return nullptr;
+}
+
+const Function* getCUDAdtor(llvm::Module& M) {
+ for (const auto& Func: M)
+if (Func.hasName() && Func.getName().startswith("__cuda_module_dtor_"))
+ return
+
+ return nullptr;
+}
+
+TEST(IncrementalProcessing, EmitCUDAGlobalInitFunc) {
+LLVMContext Context;
+CompilerInstance compiler;
+
+compiler.createDiagnostics();
+compiler.getLangOpts().CPlusPlus = 1;
+compiler.getLangOpts().CPlusPlus11 = 1;
+compiler.getLangOpts().CUDA = 1;
+
+compiler.getTargetOpts().Triple = llvm::Triple::normalize(
+llvm::sys::getProcessTriple());
+compiler.setTarget(clang::TargetInfo::CreateTargetInfo(
+ compiler.getDiagnostics(),
+ std::make_shared(
+compiler.getTargetOpts(;
+
+// To enable the generating of cuda host code, it's needs to set up the
+// auxTriple.
+llvm::Triple hostTriple(llvm::sys::getProcessTriple());
+compiler.getFrontendOpts().AuxTriple =
+hostTriple.isArch64Bit() ? "nvptx64-nvidia-cuda" : "nvptx-nvidia-cuda";
+auto targetOptions = std::make_shared();
+targetOptions->Triple = compiler.getFrontendOpts().AuxTriple;
+targetOptions->HostTriple = compiler.getTarget().getTriple().str();
+compiler.setAuxTarget(clang::TargetInfo::CreateTargetInfo(
+compiler.getDiagnostics(), targetOptions));
+
+// A fatbinary file is necessary, that the code generator generates the ctor
+// and dtor.
+auto tmpFatbinFileOrError = llvm::sys::fs::TempFile::create("dummy.fatbin");
+ASSERT_TRUE((bool)tmpFatbinFileOrError);
+auto tmpFatbinFile = std::move(*tmpFatbinFileOrError);
+compiler.getCodeGenOpts().CudaGpuBinaryFileName = tmpFatbinFile.TmpName;
+
+compiler.createFileManager();
+compiler.createSourceManager(compiler.getFileManager());
+compiler.createPreprocessor(clang::TU_Prefix);
+compiler.getPreprocessor().enableIncrementalProcessing();
+
+compiler.createASTContext();
+
+CodeGenerator* CG =
+CreateLLVMCodeGen(
+compiler.getDiagnostics(),
+"main-module",
+compiler.getHeaderSearchOpts(),
+compiler.getPreprocessorOpts(),
+compiler.getCodeGenOpts(),
+Context);
+
+compiler.setASTConsumer(std::unique_ptr(CG));
+compiler.createSema(clang::TU_Prefix, nullptr);
+Sema& S = compiler.getSema();
+
+std::unique_ptr ParseOP(new Parser(S.getPreprocessor(), S,
+ /*SkipFunctionBodies*/ false));
+Parser = *ParseOP.get();
+
+std::array M;
+M[0] = IncrementalParseAST(compiler, P, *CG, nullptr);
+ASSERT_TRUE(M[0]);
+
+M[1] = IncrementalParseAST(compiler, P, *CG, CUDATestProgram1);
+ASSERT_TRUE(M[1]);
+ASSERT_TRUE(M[1]->getFunction("_Z9cudaFunc1v"));
+
+M[2] = IncrementalParseAST(compiler, P, *CG, CUDATestProgram2);
+ASSERT_TRUE(M[2]);
+ASSERT_TRUE(M[2]->getFunction("_Z9cudaFunc2v"));
+// First code should not end up in second module:
+ASSERT_FALSE(M[2]->getFunction("_Z9cudaFunc1v"));
+
+// Make sure, that cuda ctor's and dtor's exist:
+const Function* CUDActor1 = getCUDActor(*M[1]);
+ASSERT_TRUE(CUDActor1);
+
+const Function* CUDActor2 = getCUDActor(*M[2]);
+ASSERT_TRUE(CUDActor2);
+
+const Function* CUDAdtor1 = getCUDAdtor(*M[1]);
+ASSERT_TRUE(CUDAdtor1);
+
+const Function* CUDAdtor2 = getCUDAdtor(*M[2]);
+ASSERT_TRUE(CUDAdtor2);
+
+// Compare the names of both ctor's and dtor's to check, that they are
+
