[SYCL][E2E] Find llvm-link only in SYCL compiler paths
#19847
Conversation
This patch fixes an issue found in the internal compiler: tests which use `llvm-link` failed due to the fact that `llvm-link` was taken from llorg compiler pre-installed on testing machines. This patch forces searching for ours `llvm-link`, with SYCL-related features.
dm-vodopyanov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
dm-vodopyanov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
dm-vodopyanov
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
| llvm_link = FindTool("llvm-link").resolve( | ||
| llvm_config, os.pathsep.join([config.dpcpp_bin_dir, config.llvm_tools_dir]) | ||
| ) | ||
| if not llvm_link: | ||
| lit_config.fatal("can't find `llvm-link`") | ||
|
|
There was a problem hiding this comment.
ToolSubst can accept FindTool as a command and unresolved can be set to fatal - all functionality is already there, why do we need a manual error message?
There was a problem hiding this comment.
Separate call to FindTool::resolve() is needed to tell scripts to look only in our folders, not everywhere in env on a testing machine, and I guess if llvm_link variable is None, ToolSubst on line 842 will start looking for the tool we pass as the first arg - for "llvm-link", and it will find it in the path (it will find llorg's llvm-link), and overall ToolSubst will not fail even if change unresolved= from "ignore" to "fatal" , so we need to prevent that situation by throwing error earlier, which was done on line 661.
|
@uditagarwal97, friendly ping |
Not related to this patch, submitted #19903 |
3 participants
This patch fixes an issue found in the internal compiler: tests which use
llvm-linkfailed due to the fact thatllvm-linkwas taken from llorg compiler pre-installed on testing machines. This patch forces searching for oursllvm-link, with SYCL-related features.