[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

This revision was automatically updated to reflect the committed changes. Closed by commit rGddeab07ca632: [clang-repl][CUDA] Re-land: Initial interactive CUDA support for clang-repl (authored by argentite). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite updated this revision to Diff 525572. argentite added a comment. Workaround for depending on NVPTX symbols: initialize all available targets instead. If NVPTX is not available, it will complain when we try to actually execute anything in CUDA mode. Rebased and fixed conflicts on recent

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev added a comment. In D146389#4358358 , @argentite wrote: > Added some std::move fixes for Error -> Expected conversions > > We need to figure out a solution when NVPTX backend is not enabled. > clang-repl probably should not depends on that.

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite updated this revision to Diff 524006. argentite added a comment. Added some std::move fixes for Error -> Expected conversions We need to figure out a solution when NVPTX backend is not enabled. clang-repl probably should not depends on that. Example: https://lab.llvm.org/buildbot#buil

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG80e7eed6a610: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl (authored by argentite). Changed prior to commit: https://reviews

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev accepted this revision. v.g.vassilev added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146389/new/ https://reviews.llvm.org/D146389 __

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite updated this revision to Diff 522717. argentite added a comment. Update the filenames Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146389/new/ https://reviews.llvm.org/D146389 Files: clang/include/clang/Interpreter/Interpreter.h cla

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev added a comment. This is looking good. Can you address my minor comments and run clang-format? Comment at: clang/lib/Interpreter/DeviceOffload.cpp:1 +//===-- Offload.cpp - CUDA Offloading ---*- C++ -*-===// +// v.g.vassilev

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite updated this revision to Diff 522041. argentite added a comment. Remove the copy of CodeGenOpts in CodeGeneratorImpl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146389/new/ https://reviews.llvm.org/D146389 Files: clang/include/clang/

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev added inline comments. Comment at: clang/lib/CodeGen/ModuleBuilder.cpp:39 const PreprocessorOptions &PreprocessorOpts; // Only used for debug info. -const CodeGenOptions CodeGenOpts; // Intentionally copied in. +CodeGenOptions CodeGenOpts; // Intention

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite added inline comments. Comment at: clang/lib/CodeGen/ModuleBuilder.cpp:39 const PreprocessorOptions &PreprocessorOpts; // Only used for debug info. -const CodeGenOptions CodeGenOpts; // Intentionally copied in. +CodeGenOptions CodeGenOpts; // Intentionall

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev added inline comments. Comment at: clang/lib/CodeGen/ModuleBuilder.cpp:39 const PreprocessorOptions &PreprocessorOpts; // Only used for debug info. -const CodeGenOptions CodeGenOpts; // Intentionally copied in. +CodeGenOptions CodeGenOpts; // Intention

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite updated this revision to Diff 521742. argentite marked an inline comment as done. argentite added a comment. Add some CUDA basic functionality tests. Disallow undo-ing of the initial PTU. This should fix the undo command test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev added a comment. Generally lgtm, let's extend the test coverage. Comment at: clang/lib/Interpreter/DeviceOffload.cpp:1 +//===-- Offload.cpp - CUDA Offloading ---*- C++ -*-===// +// Likewise. Comment at: c

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite updated this revision to Diff 520636. argentite marked 3 inline comments as done. argentite added a comment. Added a check to run CUDA tests only on systems with CUDA. We need some ideas for the actual tests. Rename Offload.cpp to DeviceOffload.cpp Other syntax/style fixes Repository:

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev added inline comments. Comment at: clang/lib/Interpreter/Interpreter.cpp:144-146 ClangArgv.insert(ClangArgv.end(), "-Xclang"); ClangArgv.insert(ClangArgv.end(), "-fincremental-extensions"); ClangArgv.insert(ClangArgv.end(), "-c"); Hahnfeld w

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

Hahnfeld added a comment. Some comments, but otherwise LGTM Comment at: clang/include/clang/Interpreter/Interpreter.h:43 public: + IncrementalCompilerBuilder(){}; + and this should probably be run through `clang-format`... Comment at: clang

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

SimeonEhrig added a comment. In D146389#4292984 , @tra wrote: > lib/CodeGen changes look OK to me. I can confirm the code change in CodeGen works as expected. `clang-repl` does not generate temporary files anymore, if a CUDA kernel is compiled. Compili

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

tra added a comment. lib/CodeGen changes look OK to me. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6257 + // Device code should not be at top level. + if (LangOpts.CUDA && LangOpts.CUDAIsDevice) +return; Could you give me an example of what exactly w

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite added inline comments. Comment at: clang/lib/Interpreter/Offload.cpp:1 +//===-- Offload.cpp - CUDA Offloading ---*- C++ -*-===// +// v.g.vassilev wrote: > argentite wrote: > > Hahnfeld wrote: > > > v.g.vassilev wrote: > > > > Ho

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev added a comment. Generally, looks good to me. I'd like to wait for @Hahnfeld and @tra's feedback at least for another week before merging. @dblaikie, I know that generally we do not want to run tests on the bots and that makes testing quite hard for this patch. Do you have a sugges

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite updated this revision to Diff 516061. argentite added a comment. Herald added a subscriber: ormris. Use virtual file system to store CUDA fatbinaries in memory Adapted Interpreter tests to use the CompilerBuilder Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev added a reviewer: dblaikie. v.g.vassilev added a subscriber: dblaikie. v.g.vassilev added a comment. I am adding @dblaikie as he might have ideas how to test this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146389/new/ https:/

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite added inline comments. Comment at: clang/lib/Interpreter/Offload.cpp:90-91 + PTXCode += '\0'; + while (PTXCode.size() % 8) +PTXCode += '\0'; + return PTXCode.str(); Hahnfeld wrote: > Is padding to 8 bytes a requirement for PTX? Maybe add a coment

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite updated this revision to Diff 512521. argentite marked 3 inline comments as done. argentite added a comment. Combined IncrementalCompilerBuilder and IncrementalCudaCompilerBuilder Added --cuda-path support Use sys::fs::createTemporaryFile() instead of hardcoding the path Other minor refa

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

SimeonEhrig added a comment. Except using an in-memory solution for generated fatbin code, the code looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146389/new/ https://reviews.llvm.org/D146389 _

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

Hahnfeld added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:296 } + LinkModules.clear(); return false; // success v.g.vassilev wrote: > argentite wrote: > > Hahnfeld wrote: > > > This looks like a change that has implication

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:296 } + LinkModules.clear(); return false; // success argentite wrote: > Hahnfeld wrote: > > This looks like a change that has implications beyond support for C

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite added a comment. In D146389#4243900 , @tra wrote: >> Initial interactive CUDA support for clang-repl > > What should a user expect to be supported, functionality wise? I assume it > should cover parsing and compilation. I'm not so sure about th

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

Hahnfeld added inline comments. Comment at: clang/include/clang/Interpreter/Interpreter.h:43-44 public: static llvm::Expected> create(std::vector &ClangArgv); + static llvm::Expected> If I understand the change correctly, the "old" `create` function on it

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

SimeonEhrig added inline comments. Comment at: clang/tools/clang-repl/ClangRepl.cpp:135 +std::move(CI), std::move(DeviceCI), OffloadArch, +"/tmp/clang-repl.fatbin")); + v.g.vassilev wrote: > To cover the case where platforms have no `/tmp` we coul

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev added inline comments. Comment at: clang/tools/clang-repl/ClangRepl.cpp:137 + +ExitOnErr(Interp->LoadDynamicLibrary("libcudart.so")); + } else tra wrote: > v.g.vassilev wrote: > > tra wrote: > > > Is there any doc describing the big picture appr

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

tra added a comment. > Initial interactive CUDA support for clang-repl What should a user expect to be supported, functionality wise? I assume it should cover parsing and compilation. I'm not so sure about the execution. Should it be expected to actually launch kernels, or will that come in a f

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev added a subscriber: SimeonEhrig. v.g.vassilev added inline comments. Comment at: clang/tools/clang-repl/ClangRepl.cpp:137 + +ExitOnErr(Interp->LoadDynamicLibrary("libcudart.so")); + } else tra wrote: > Is there any doc describing the big picture

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

tra added inline comments. Comment at: clang/tools/clang-repl/ClangRepl.cpp:27 +static llvm::cl::opt CudaEnabled("cuda", llvm::cl::Hidden); +static llvm::cl::opt OffloadArch("offload-arch", llvm::cl::Hidden); + Where will clang-repl find CUDA headers? Generally

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

v.g.vassilev added a comment. Thanks for working on this! Comment at: clang/lib/Interpreter/Offload.cpp:1 +//===-- Offload.cpp - CUDA Offloading ---*- C++ -*-===// +// How about `DeviceOffload.cpp`? Comment at: clang/

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

argentite created this revision. Herald added subscribers: mattd, carlosgalvezp, yaxunl. Herald added a project: All. argentite updated this revision to Diff 507049. argentite added a comment. argentite updated this revision to Diff 510808. argentite edited the summary of this revision. argentite a