gemini-code-assist[bot] commented on code in PR #19594:
URL: https://github.com/apache/tvm/pull/19594#discussion_r3285778938
##########
cmake/modules/contrib/CLML.cmake:
##########
@@ -68,17 +68,17 @@ if(USE_CLML_GRAPH_EXECUTOR)
list(APPEND EXTERN_CLML_COMPUTE_LIB ${CLML_PATH}/lib/libOpenCL.so
${CLML_PATH}/lib/libOpenCL_system.so)
endif()
endif()
- list(APPEND TVM_RUNTIME_LINKER_LIBS ${EXTERN_CLML_COMPUTE_LIB})
- list(APPEND RUNTIME_SRCS ${CLML_CONTRIB_SRC})
+ file(GLOB RUNTIME_OPENCL_SRCS src/runtime/opencl/*.cc)
+ add_library(tvm_clml_objs OBJECT ${CLML_CONTRIB_SRC}
${RUNTIME_OPENCL_SRCS})
Review Comment:
This change duplicates the OpenCL runtime source files into the
`tvm_clml_objs` object library, which is then linked into
`libtvm_runtime_extra.so`. Since `libtvm_runtime_opencl.so` also contains these
same symbols, loading both libraries will result in duplicate symbol
definitions and potential conflicts in the global TVM registry (e.g., multiple
registrations of the OpenCL device API).
Instead of duplicating sources, `tvm_runtime_extra` should link against the
`tvm_runtime_opencl` target. Since `CLML.cmake` is included before
`OpenCL.cmake`, you may need to move the
`target_link_libraries(tvm_runtime_extra ...)` call for OpenCL to the end of
the main `CMakeLists.txt`, similar to how it is handled for CUDA at line 513.
##########
ci/scripts/jenkins/s3.py:
##########
@@ -170,7 +170,13 @@ def s3(source: str, destination: str, recursive: bool) ->
list[str]:
if item != ".":
source = s3_path + "/" + item
recursive = False
- stdout = s3(source=source, destination=item, recursive=recursive)
+ try:
+ stdout = s3(source=source, destination=item,
recursive=recursive)
+ except Exception:
+ # Optional artifacts (e.g. per-backend device runtime DSOs)
may not
+ # exist in S3 when the build config didn't produce them. Skip
silently.
+ logging.warning(f"Download failed for {item}, skipping (may be
optional)")
+ continue
Review Comment:
Catching a generic `Exception` here is quite broad and might mask legitimate
infrastructure or network issues during CI runs. It would be safer to catch
specific exceptions related to missing files (e.g., by checking the exit code
or output of the `s3` command for a 404/NoSuchKey error) to ensure that only
truly optional artifacts are skipped while real failures are still reported.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
