-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RHEL SBSA Build #99
RHEL SBSA Build #99
Conversation
CMakeLists.txt
Outdated
@@ -52,6 +52,8 @@ endif() | |||
set(TRITON_TENSORRT_BACKEND_LIBNAME triton_tensorrt) | |||
set(TRITON_TENSORRT_BACKEND_INSTALLDIR ${CMAKE_INSTALL_PREFIX}/backends/tensorrt) | |||
|
|||
set(CMAKE_PREFIX_PATH "/usr/local/cuda/targets/sbsa-linux/lib;/usr/local/cuda/targets/x86_64-linux/lib") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Seems like an anti-pattern to have multiple arches in a PATH variable but this is also concise. Maybe add a comment making note that if any other paths need to be added, this should be split.
-
Does this overwrite any
CMAKE_PREFIX_PATH
s which are added on the command line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Circling back on this, I don't like the approach either. I'll set it conditionally.
- Good catch!
# results when running with command line arg "-DCMAKE_PREFIX_PATH:STRING=/hello;/world"
foreach(PATH ${CMAKE_PREFIX_PATH})
message("PATH: ${PATH}")
endforeach()
# before:
PATH: /usr/local/cuda/targets/x86_64-linux/lib
# after:
PATH: /hello
PATH: /world
PATH: /usr/local/cuda/targets/x86_64-linux/lib
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Semicolon-separated list of directories specifying installation prefixes to be searched by the find_package(), find_program(), find_library(), find_file(), and find_path() commands. Each command will add appropriate subdirectories (like bin, lib, or include) as specified in its own documentation.
-
In other words if you don't mess with providers, and not planing to provide different architecture CMake will not mess up with dependencies.
There why suppose condition will be redundant by serving same purpose. -
Given argument from command line should extend not override configuration within CMAKE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you suggesting that we disallow users from setting this variable at all and go back to overwriting it?
e.g., aarch64: set(CMAKE_PREFIX_PATH ${CMAKE_PREFIX_PATH} "/usr/local/cuda/targets/sbsa-linux/lib")
I can't say that if feels right to do that. If a user is building in their own environment, they should be able to set this variable as-needed.
Goal: Incorporate SBSA builds/tests and extended the smoke tests for x86 and SBSA to include L0_sequence_batcher.
Server changes: triton-inference-server/server#7595
PyTorch Backend changes: triton-inference-server/pytorch_backend#139
TensorFlow Backend changes: triton-inference-server/tensorflow_backend#106