Add CMake support to build the libraries

[gcuendet]

Description

This PR adds the CMake support to build the libraries and the torchtrtc executable. It also generates a torchtrtConfig.cmake, in order to be able to consume these libraries with cmake targets.

The major advantage of the CMake support as introduced in this PR is the added ability to compile for windows as well, fixing the export of symbols, and generating both the *.dll and the *.lib libraries. To export the symbols, I reused the existing TORCHTRT_API macro, redefining it for windows, using CMake. The consumption of the dynamic libraries (both on linux and windows) is demonstrated by building the executable torchtrtc[.exe]. This has been tested locally with MSVC 2019 (19.29.30143.0), libtorch 1.11 (1.11.0+cu113), CUDA 11.6, cuDNN 8.2.1, and TensorRT 8.2.4.2.
A secondary advantage is the handling of dependencies. These can be installed anywhere (not necessarily globally on the system) and by any mean and compiling the lib does not require editing config file anymore, instead, a few CMake variables are enough to have the dependencies correctly found and used. (for cuDNN and TensorRT, most of the magic happens in the CMake finders in cmake/Modules).

This PR only adds partial CMake support to Torch-TensorRT (only for the libs and the executable torchtrtc). I started working on the unit tests as well and could consider working on adding the python support as well, if there is a clear interest. I still think that being able to compile the libraries (also on windows) has value in itself, and that's the reason for this PR.

I didn't add any documentation about how to compile using CMake, but that should probably be done. A bit of guidance on where and how this should be best done would be appreciated.

With this PR, I also would like to trigger the discussion around CMake and/or windows support (which, technically, are obviously totally independent). The kind of questions I have are:

• Is there a CI for this lib (I am assuming there is)? Could we imagine also compiling the lib with Cmake on the CI, to check that any new addition does not break it in the future? If not, how do you see the maintenance of the feature?
• Same question for windows: I am assuming the lib is not compiled on windows on the CI currently. Can that be added? Possibly with CMake?

Thanks for reviewing and looking forward to answer questions 😃

Type of change

• New feature (non-breaking change which adds functionality)

Checklist:

• My code follows the style guidelines of this project (You can use the linters)
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas and hacks
• I have made corresponding changes to the documentation
• I have added tests to verify my fix or my feature
• New and existing unit tests pass locally with my changes

Signed-off-by: Gabriel Cuendet gabriel.cuendet@gmail.com

[narendasan]

@gcuendet Can you sign your commits?

[narendasan]

This PR only adds partial CMake support to Torch-TensorRT (only for the libs and the executable torchtrtc). I started working on the unit tests as well and could consider working on adding the python support as well, if there is a clear interest. I still think that being able to compile the libraries (also on windows) has value in itself, and that's the reason for this PR.

I think adding a --use-cmake flag to the setup.py to use the CMake system might be a good addition. I am not too sure about the unit tests as we are still working to get bazel working on Windows and I would rather keep the alternative build systems as light weight as possible. Guess it depends on how much extra work it is to add cmake tests.

I didn't add any documentation about how to compile using CMake, but that should probably be done. A bit of guidance on where and how this should be best done would be appreciated.

//docsrc/tutorials/installation.rst would be the spot for this. You can make a CMake section or something.

Is there a CI for this lib (I am assuming there is)? Could we imagine also compiling the lib with Cmake on the CI, to check that any new addition does not break it in the future? If not, how do you see the maintenance of the feature?

cc: @andi4191, I think we could just try to build it and just report if succeeds or not

Same question for windows: I am assuming the lib is not compiled on windows on the CI currently. Can that be added? Possibly with CMake?

Windows CI is part of our testing plan down the road as support matures.

[facebook-github-bot]

Hi @gcuendet!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[gcuendet]

@gcuendet Can you sign your commits?

I thought I did. If you look at my commit messages, they are all signed. What is it that is missing? Is there anything else to do than just -s when committing?

[gcuendet]

Hi @gcuendet!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

This has come as kind of a surprise. In the CONTRIBUTING.md, there is no mention of a Meta CLA. I had the CLA included there reviewed by the legal department of my company before considering to contribute, but of course, now that the CLA has changed, I'll need a new round of reviewing. This might take time 😞

[narendasan]

@gcuendet Yeah sorry about that, it recently changed (like late last week) as we are now working with Meta on co-developing this project so the requirements to upstream code now includes the Meta CLA instead of DCO (https://pytorch.org/blog/a-contributor-license-agreement-for-pytorch/). Hopefully we can still make progress on review while you get approval on your side

[narendasan]

We probably want to export the internal APIs to register custom converters as well (see

TensorRT/examples/custom_converters/elu_converter/csrc/elu_converter.cpp

Line 6 in 9e5fedf

auto actelu = torch_tensorrt::core::conversion::converters::RegisterNodeConversionPatterns().pattern(

for how it is used)

[gcuendet]

We probably want to export the internal APIs to register custom converters as well (see

What do you mean exactly with that?
Do you mean that there should be a (public) cmake target that allows a consumer to link against internal libraries and include their headers (typically on linux)? Or do you mean that the symbols should be exported (on windows)?
Is there any plan to refactor that part and maybe better expose that functionality in the public API?

[narendasan]

We probably want to export the internal APIs to register custom converters as well (see

What do you mean exactly with that? Do you mean that there should be a (public) cmake target that allows a consumer to link against internal libraries and include their headers (typically on linux)? Or do you mean that the symbols should be exported (on windows)? Is there any plan to refactor that part and maybe better expose that functionality in the public API?

I mean export the symbols. I don't think we have any plans to change this. We want to keep it separate from the user level API most people use, however it is really just this file that people would use https://github.com/pytorch/TensorRT/blob/master/core/conversion/converters/converters.h

[andi4191]

Running into issues with linux.

[gcuendet]

@gcuendet We are currently testing this internally. Hopefully it should be wrapped up soon.

@narendasan how is the internal testing going? Is there anything I can help with?
I tried to request access to your slack organisation (filled in the form), but so far without success. If there is anything that we should discuss directly, maybe we can find another way?
Let me know.

[peri044]

I'm facing errors while running this

cmake -S. -B<build directory> \
    [-DCMAKE_MODULE_PATH=cmake/Module] \
    [-DTorch_DIR=<path to libtorch>/share/cmake/Torch] \
    [-DTensorRT_ROOT=<path to TensorRT>] \
    [-DCMAKE_BUILD_TYPE=Debug|Release]

The exact command I'm using

cmake -S . -B build/ -DTensorRT_LIBRARY=/home/dperi/Downloads/public_trt/TensorRT-8.2.4.2/lib -DTorch_DIR=/home/dperi/Downloads/TensorRT/bazel-TensorRT/external/libtorch_pre_cxx11_abi/share/cmake/Torch/  -DTensorRT_INCLUDE_DIR=/home/dperi/Downloads/public_trt/TensorRT-8.2.4.2/include/ -DcuDNN_LINK_LIBRARY=/home/dperi/Downloads/cudnn/cudnn-8.2.1/lib64/ -DcuDNN_INCLUDE_DIRS=/home/dperi/Downloads/cudnn/cudnn-8.2.1/include/

Errors:

CMake Error at cpp/CMakeLists.txt:96 (add_library):
  Target "torchtrt_runtime" links to target "TensorRT::nvinfer_plugin" but
  the target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?


CMake Error at cpp/CMakeLists.txt:124 (add_library):
  Target "torchtrt_plugins" links to target "TensorRT::nvinfer_plugin" but
  the target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?


CMake Error at core/plugins/CMakeLists.txt:2 (add_library):
  Target "core_plugins" links to target "TensorRT::nvinfer_plugin" but the
  target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?

@gcuendet anything I'm missing here ?

[gcuendet]

I'm facing errors while running this

cmake -S. -B<build directory> \
    [-DCMAKE_MODULE_PATH=cmake/Module] \
    [-DTorch_DIR=<path to libtorch>/share/cmake/Torch] \
    [-DTensorRT_ROOT=<path to TensorRT>] \
    [-DCMAKE_BUILD_TYPE=Debug|Release]

The exact command I'm using

cmake -S . -B build/ -DTensorRT_LIBRARY=/home/dperi/Downloads/public_trt/TensorRT-8.2.4.2/lib -DTorch_DIR=/home/dperi/Downloads/TensorRT/bazel-TensorRT/external/libtorch_pre_cxx11_abi/share/cmake/Torch/  -DTensorRT_INCLUDE_DIR=/home/dperi/Downloads/public_trt/TensorRT-8.2.4.2/include/ -DcuDNN_LINK_LIBRARY=/home/dperi/Downloads/cudnn/cudnn-8.2.1/lib64/ -DcuDNN_INCLUDE_DIRS=/home/dperi/Downloads/cudnn/cudnn-8.2.1/include/

Errors:

CMake Error at cpp/CMakeLists.txt:96 (add_library):
  Target "torchtrt_runtime" links to target "TensorRT::nvinfer_plugin" but
  the target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?


CMake Error at cpp/CMakeLists.txt:124 (add_library):
  Target "torchtrt_plugins" links to target "TensorRT::nvinfer_plugin" but
  the target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?


CMake Error at core/plugins/CMakeLists.txt:2 (add_library):
  Target "core_plugins" links to target "TensorRT::nvinfer_plugin" but the
  target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?

@gcuendet anything I'm missing here ?

TL;DR
Did you try with the actual documented command? (i.e. setting -DTensorRT_ROOT=<path to TensorRT>)

cmake -S . -B build/ -DTensorRT_ROOT=/home/dperi/Downloads/public_trt/TensorRT-8.2.4.2/ -DTorch_DIR=/home/dperi/Downloads/TensorRT/bazel-TensorRT/external/libtorch_pre_cxx11_abi/share/cmake/Torch/ -DcuDNN_ROOT_DIR=/home/dperi/Downloads/cudnn/cudnn-8.2.1/

In more details:

You are getting the following error:

CMake Error at cpp/CMakeLists.txt:96 (add_library):
  Target "torchtrt_runtime" links to target "TensorRT::nvinfer_plugin" but
  the target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?

As the message says, CMake cannot find a target, specifically one of TensorRT targets (TensorRT::nvinfer_plugin). Apparently, you tried to manually specify -DTensorRT_LIBRARY and -DTensorRT_INCLUDE_DIR but this PR relies on CMake targets rather than just paths to libraries and include directories (which is sometimes refer to as "modern" cmake).
Since TensorRT and cuDNN do not come with CMake finders by default (allowing to define these targets), I provide these finders in cmake/Module. If TensorRT and/or cuDNN are not installed in a default location (as in your case), the only thing these finders need to define the targets is the path to the root of the respective install locations, specified with -DTensorRT_ROOT and/or -DcuDNN_ROOT_DIR.

I hope this helps. Let me know if something is unclear.

[peri044]

Thanks @gcuendet. It works now.
I tested on ubuntu and it works. Following things need to be documented in the CMake section of installation.rst so that the expectation is clear and these can be addressed in the future update

1. Looks like the python api cannot be built using cmake now (as naren mentioned --use-cmake)
2. CMake support for tests is missing.
   cc: @narendasan

[peri044]

Following things need to be documented in the CMake section of installation.rst so that the expectation is clear and these can be addressed in the future update

Looks like the python api cannot be built using cmake now (as naren mentioned --use-cmake)
CMake support for tests is missing.

[gcuendet]

Following things need to be documented in the CMake section of installation.rst so that the expectation is clear and these can be addressed in the future update

Looks like the python api cannot be built using cmake now (as naren mentioned --use-cmake) CMake support for tests is missing.

I added one more line in the CMake section of the installation.rst, as requested.

Note that, as offered when I initially opened this PR, I can work on the python api compilation with CMake or the unit test support, but I will not do that as part of this PR. I first would like to see this one merged.

Also, if you think there is a need for these additional things and would welcome my contributions for these (as opposed to doing them internally), I would like to be able to better communicate with you (typically by having access to your slack org).

[narendasan]

I would like to be able to better communicate with you (typically by having access to your slack org).

@gcuendet We are on the PyTorch slack which you can join through this form https://docs.google.com/forms/d/e/1FAIpQLSeADnUNW36fjKjYzyHDOzEB_abKQE9b6gqqW9NXse6O0MWh0A/viewform