[SYCL-RTC] Use ToolInvocation directly instead of via ClangTool
#19922
Conversation
We only run SYCL JIT on a single TU at a time, so using `ClangTool` is a bit awkward, as it is primarily used to run the same action across a set of files: https://github.com/intel/llvm/blob/357f96b7e19d8acb972eb2f1fb276dbc6aa2060b/clang/include/clang/Tooling/Tooling.h#L310-L317 Using `ToolInvocation` better matches our scenario of always doing a single clang invocation: https://github.com/intel/llvm/blob/357f96b7e19d8acb972eb2f1fb276dbc6aa2060b/clang/include/clang/Tooling/Tooling.h#L244-L245 Another benefit is that we have more control over the virtual file system which I'm planning to use in a subsequent PR to have the SYCL toolchain headers distributed inside `libsycl-jit.so` and then put into an `llvm::vfs::InMemoryFileSystem` once to be re-used across all compilation queries. I'm also simplifying the inheritance scheme around `clang::ToolAction`. Instead of having both hashing/compiling doing that, I'm providing a single helper that accepts a reference to the `FrontendAction` that can be kept on the caller's stack, reducing the amount of boilerplate helpers necessary, i.e. `RTCToolActionBase`/`GetSourceHashAction`/`GetLLVMModuleAction` before vs. single `SYCLToolchain::Action` after.
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| } else { | ||
| return createStringError(BuildLog); | ||
| } |
There was a problem hiding this comment.
| } else { | |
| return createStringError(BuildLog); | |
| } | |
| } | |
| return createStringError(BuildLog); |
| } else { | ||
| return createStringError("Calculating source hash failed"); | ||
| } |
There was a problem hiding this comment.
| } else { | |
| return createStringError("Calculating source hash failed"); | |
| } | |
| } | |
| return createStringError("Calculating source hash failed"); |
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| const std::string &DPCPPRoot, BinaryFormat Format, | ||
| SmallVectorImpl<std::string> &CommandLine) { | ||
| static std::vector<std::string> | ||
| createCommandLine(const InputArgList &UserArgList, std::string_view DPCPPRoot, |
There was a problem hiding this comment.
what will the command line typically be? clang++ -c <source1..N> ? Might be nice to document it.
There was a problem hiding this comment.
Still not sure what this pre-existing code does 😆 Currently it would be
/full/path/to/dpcpp/installation/on/the/actual/file/system/bin/clang++ <magic> rtc_N.cpp
I'm trying to drop the requirement to have an actual installation and turn it to this instead:
/fake_path/bin/clang++ <magic> rtc_N.cpp
in #19924
There was a problem hiding this comment.
@jopperm would know more about this, but the correct path might be relevant for determining the location of libraries and headers that are part of the DPC++ distribution.
There was a problem hiding this comment.
Yes, IIRC setting the argv0 of the invocation to the actual clang++ binary made the detection of include paths work automatically. Some logic might still be sensitive to the exe being called clang++, but if you set the include paths explicitly anyways, a fake path should be fine.
There was a problem hiding this comment.
Yes, it is. The virtual in-memory filesystem has to have the files in /fake_path/include/sycl too.
5 participants
We only run SYCL JIT on a single TU at a time, so using
ClangToolis a bit awkward, as it is primarily used to run the same action across a set of files:llvm/clang/include/clang/Tooling/Tooling.h
Lines 310 to 317 in 357f96b
Using
ToolInvocationbetter matches our scenario of always doing a single clang invocation:llvm/clang/include/clang/Tooling/Tooling.h
Lines 244 to 245 in 357f96b
Another benefit is that we have more control over the virtual file system which I'm planning to use in a subsequent PR to have the SYCL toolchain headers distributed inside
libsycl-jit.soand then put into anllvm::vfs::InMemoryFileSystemonce to be re-used across all compilation queries.I'm also simplifying the inheritance scheme around
clang::ToolAction. Instead of having both hashing/compiling doing that, I'm providing a single helper that accepts a reference to theFrontendActionthat can be kept on the caller's stack, reducing the amount of boilerplate helpers necessary, i.e.RTCToolActionBase/GetSourceHashAction/GetLLVMModuleActionbefore vs. singleSYCLToolchain::Actionafter.