search

mlc-ai / mlc-llm

Universal LLM Deployment Engine with ML Compilation
https://llm.mlc.ai/
Apache License 2.0
19.24k stars 1.58k forks source link

Header file of TVM is referred not from `include` but `src`. #2964

Closed grf53 closed 1 month ago

grf53 commented 1 month ago

Hello.

Issue

While using the mlc-llm, I found that a header file in TVM is referred through its hard-coded relative path like: https://github.com/mlc-ai/mlc-llm/blob/7c1f3c3cbb41d012a033e7158c281a652982e0d8/cpp/json_ffi/image_utils.cc#L5

It is because the file base64.h is not in include directory, but in src directory of TVM even though it is referred by other projects like 'mlc-llm'.

This situation makes it difficult to use TVM from other locations than 3rdparty/tvm.

c.f.) In TVM(relax) repository itself, there is a part that using the same way to import base64.h(here).

What is expected

base64.h should be moved from https://github.com/mlc-ai/relax/tree/mlc/src/support to https://github.com/mlc-ai/relax/tree/mlc/include/tvm/support. And along with that, all the #include directives including that header file should be modified. I locally moved it and changed #include "../../3rdparty/tvm/src/support/base64.h" to #include <tvm/support/base64.h>, then the problem resolved.

Or, is there any special reason not to do that? If so, kindly let me know. Thanks.

MasterJH5574 commented 1 month ago

Thank you @grf53 for bringing this up. I guess the functions in base64.h are not designed to be public interfaces of TVM so people didn't put it under include/ and we have to import with with #include "...".

This situation makes it difficult to use TVM from other locations than 3rdparty/tvm.

Regarding this, I wonder whether you have all the 3rdparty repo cloned recursively? For my personal use, though I have a standalone directory for TVM, I still keep 3rdparty/tvm there so such include doesn't have any issue.