Page MenuHomePhabricator

[AMDGPU][OpenMP] Enable Libomptarget runtime tests
Needs ReviewPublic

Authored by pdhaliwal on Wed, Mar 31, 5:19 AM.

Details

Summary

This patch enables the libomptarget runtime tests for amdgpu target.
Currently, user will have to explicitly pass -DLIBOMPTARGET_ENABLE_EXPERIMENTAL_AMDGCN_TARGET=On
to enable building on amdgpu plugin and deviceRTL. There are around 12 tests
which are failing either due to missing printf or malloc.

Diff Detail

Event Timeline

pdhaliwal created this revision.Wed, Mar 31, 5:19 AM
pdhaliwal requested review of this revision.Wed, Mar 31, 5:19 AM
Herald added a project: Restricted Project. · View Herald Transcript

I guess there's no XFAIL equivalent here? In that case we should probably leave off the RUN line for the tests that can't work, as they'll otherwise break the build once an amdgpu CI machine goes live

openmp/libomptarget/CMakeLists.txt
40

I don't understand this string. Maybe in '-march=$LIBOMPTARGET_AMDGCN_TEST_TARGET'? Seems bad to be hard coding a gfx908 here, presumably it should be whatever is in the system running the tests

openmp/libomptarget/deviceRTLs/CMakeLists.txt
12

would rather enable this unconditionally, which works today except for people who build with the amdgpu target disabled. See D98746

openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt
94 ↗(On Diff #334417)

This we can patch separately. Maybe also drop the gfx7? I don't think it's being tested, and if it's broken it would be friendlier to not build it by default.

openmp/libomptarget/plugins/CMakeLists.txt
80 ↗(On Diff #334417)

This seems to be a regression, why not continue building the plugin unconditionally?

openmp/libomptarget/test/lit.site.cfg.in
19 ↗(On Diff #334417)

this seems very inconvenient for the tests, I think we should handle missing march by looking at the system we're running the tests on

pdhaliwal added inline comments.Wed, Mar 31, 7:34 AM
openmp/libomptarget/CMakeLists.txt
40

Its just a default cmake option value. Since there is no automatic march detection, I thought of having it configurable from command line. I will need dig around for detecting system gpu.

openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt
94 ↗(On Diff #334417)

I will create a separate patch for it.

openmp/libomptarget/plugins/CMakeLists.txt
80 ↗(On Diff #334417)

This was causing issues when plugin was always getting built but deviceRTL not. When plugin is built, the runtime tests will be executed which are going to fail.

openmp/libomptarget/test/lit.site.cfg.in
19 ↗(On Diff #334417)

Agreed. This seems like a problem which could be solved by Driver/ToolChain.

pdhaliwal updated this revision to Diff 336060.Thu, Apr 8, 4:44 AM
  • Addressed review comments.