[SYCL] Separate jobs in sycl-post-link tool #854
Conversation
|
|
||
| if (BaseOutputFilename == "-") | ||
| BaseOutputFilename = "a.out"; | ||
|
|
||
| saveResults(ResultModules, ResultSymbolsLists); | ||
| if (PerformSplit) { |
There was a problem hiding this comment.
please add a comment that the default usage model is that the tool is called twice with the same inputs because clang does not support actions with multiple outputs, hence this workaround with ir/txt generation splitting. It should not bring much extra overhead because parseIRFile and collectKernelsSet are small (would be good to estimate) compared to either of
splitModule(*M.get(), GlobalsSet, ResultModules);
saveResultModules(ResultModules);
and
collectSymbolsLists(GlobalsSet, ResultSymbolsLists);
saveResultSymbolsLists(ResultSymbolsLists);
| std::vector<std::string> &ResSymbolsLists) { | ||
| for (auto &It : KernelsSet) { | ||
| std::string SymbolsList; | ||
| for (auto &F : It.second) { |
There was a problem hiding this comment.
Technically it is a reduction algorithm such as std::accumulate...
Then why not just declaring SymbolsList to be a Twine to avoid the string conversions back-and-forth?
There was a problem hiding this comment.
Not sure. According to LLVM docs, Twine is a temporary object and shouldn't be stored. I use the SymbolsList next...
https://llvm.org/doxygen/classllvm_1_1Twine.html
There was a problem hiding this comment.
Agree with @Fznamznon. += can't be done on Twines, AFAIK.
There was a problem hiding this comment.
Ah yes, it looks like the Twine API is quite lower level. Sorry for the noise. :-(
Fznamznon
force-pushed
the
private/mpodchis/improve-post-link
branch
from
7c6817c to
d6ddae2
Compare
| // Input parameter KernelsSet is a map containing groups of kernels with same | ||
| // values in the sycl-module-id attribute. | ||
| // ResSymbolsLists vector is output parameter. | ||
| // Collects set of kernel names for each group of kernels. |
There was a problem hiding this comment.
I'm not what does 'collects' mean here, but I looks like this function converts std::map<std::string, std::vector<Function *>> into std::vectorstd::string.
There was a problem hiding this comment.
Tried to improve the comment.
Clang driver will invoke sycl-post-link twice: one time for IR module split and one time for entries list generation. This patch makes possible do not perform same job twice in this case. Signed-off-by: Mariya Podchishchaeva <[email protected]>
Signed-off-by: Mariya Podchishchaeva <[email protected]>
Signed-off-by: Mariya Podchishchaeva <[email protected]>
Signed-off-by: Mariya Podchishchaeva <[email protected]>
Signed-off-by: Mariya Podchishchaeva <[email protected]>
Fznamznon
force-pushed
the
private/mpodchis/improve-post-link
branch
from
d6ddae2 to
841d499
Compare
| for (size_t I = 0; I < ResModules.size(); ++I) { | ||
| std::error_code EC; | ||
| std::string CurOutFileName = BaseOutputFilename + "_" + | ||
| std::to_string(NumOfFile) + | ||
| std::string CurOutFileName = BaseOutputFilename + "_" + std::to_string(I) + |
There was a problem hiding this comment.
Is there an issue if the user is compiling toto.cpp and toto_0.cpp at the same time?
I guess it would be in 2 different temporary directories and thus no possible conflict?
Clang driver will invoke sycl-post-link twice: one time for IR module
split and one time for entries list generation.
This patch makes possible do not perform same job twice in this case.
Signed-off-by: Mariya Podchishchaeva [email protected]