[libclc] Allow default path when looking for llvm-spirv #126071
Conversation
This is an external tool, so I don't think there is an expectation that it has to be in the LLVM tools bindir. It may also be in the default system bindir (which is not necessarily the same).
frasercrmck
left a comment
frasercrmck
left a comment
There was a problem hiding this comment.
I get where this is coming from, thanks.
However, note that with this change, the priority is changed so that llvm-spirv will be picked up from the PATH environment variable before LLVM_TOOLS_BINARY_DIR.
See the documentation for find_program, when NO_DEFAULT_PATH is provided:
- step 5: Search the standard system environment variables
- step 7: Search the paths specified by the PATHS option (...)
I'm not too concerned by the other steps as they're cmake-specific options or variables.
However, this does appear to present a change to users (would re-configuring change the location of the llvm-spirv binary in such scenarios?).
Should LLVM_TOOLS_BINARY_DIR be made a HINT so that it gets searched in step 4 before system paths?
|
Good point, I've changed this to use HINTS instead of PATHS. I'm not really sure which one is more "correct" in this context, but sticking closer to the previous behavior seems like a good idea... |
|
/cherry-pick 26ecddb |
|
/pull-request #126201 |
This is an external tool, so I don't think there is an expectation that it has to be in the LLVM tools bindir. It may also be in the default system bindir (which is not necessarily the same). (cherry picked from commit 26ecddb)
This is an external tool, so I don't think there is an expectation that it has to be in the LLVM tools bindir. It may also be in the default system bindir (which is not necessarily the same).
3 participants
This is an external tool, so I don't think there is an expectation that it has to be in the LLVM tools bindir. It may also be in the default system bindir (which is not necessarily the same).