[hip][caffe2] Enable detectron on AMD GPU #17862
Conversation
modules/detectron/CMakeLists.txt
Outdated
| @@ -19,6 +20,17 @@ if (BUILD_CAFFE2_OPS) | |||
| if (MSVC) | |||
| install(FILES $<TARGET_PDB_FILE:caffe2_detectron_ops_gpu> DESTINATION lib OPTIONAL) | |||
| endif() | |||
| elseif(USE_ROCM) | |||
| set(Caffe2_HIP_INCLUDES ${Caffe2_HIP_INCLUDES} ${CMAKE_CURRENT_SOURCE_DIR}) | |||
There was a problem hiding this comment.
Add current dir to include dirs list is not desired, instead we should change the includes in the source files to use path relative to the project root (i.e. #include "modules/detectron/xxx.h" instead of #include "xxx.h")
modules/detectron/ps_roi_pool_op.cu
Outdated
| @@ -123,8 +123,13 @@ __global__ void PSRoIPoolForward( | |||
| roundf(offset_bottom_rois[4]) + 1.) * spatial_scale; | |||
|
|
|||
| // Force too small ROIs to be 1x1 | |||
| #ifdef __HIP_PLATFORM_HCC__ | |||
| T roi_width = fmax(roi_end_w - roi_start_w, 0.1); // avoid 0 | |||
There was a problem hiding this comment.
use c10::cuda::compat::max instead, which has the right dispatch to fmax and fmaxf based on the instantiated template type. (and avoid ifdef)
tools/amd_build/build_amd.py
Outdated
| @@ -63,6 +63,7 @@ | |||
| "caffe2/utils/*", | |||
| "c10/cuda/*", | |||
| "c10/cuda/test/CMakeLists.txt", | |||
| "modules/detectron/*", | |||
There was a problem hiding this comment.
We can enable the entire "modules" subdirectory here, atm only detectron module has gpu files and when people add new gpu files we want to automatically enable in rocm.
|
Thanks @bddppq , updated the pull request. Please review the changes. |
There was a problem hiding this comment.
@bddppq has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
No description provided.