This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][Libomptarget] Delete g_atl_machine global
ClosedPublic

Authored by pdhaliwal on Jun 22 2021, 3:01 AM.

Details

Summary

With uses of g_atl_machine gone, a significant portion of dead
code has been removed.

This patch depends on D104691 and D104695.

Diff Detail

Event Timeline

pdhaliwal created this revision.Jun 22 2021, 3:01 AM
pdhaliwal requested review of this revision.Jun 22 2021, 3:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 22 2021, 3:01 AM

That is a lot of code to throw away, very nice. Will take a closer look at the details shortly

openmp/libomptarget/plugins/amdgpu/impl/atmi_runtime.h
12

Maybe "hsa_ext_amd.h" to match the above "hsa.h". We're not consistent with this across the library, which is probably worse than using <> everywhere and worse than using "" everywhere.

I can't think of a compelling reason why the system search paths should be preferred for hsa, so would lean towards using "" everywhere

Forgot about this, apologies. Is it still current?

I need to rebase this.

openmp/libomptarget/plugins/amdgpu/impl/data.cpp
26

removing this switch gets rid of the last build warning I see locally

JonChesterfield accepted this revision.Aug 18 2021, 7:45 PM

This works for me when running locally. The reduction in code is excellent but somewhere past what I can review efficiently. Thank you for periodically rebasing on main.

This revision is now accepted and ready to land.Aug 18 2021, 7:45 PM

Oh sorry, forgot about this. Landing it now.

This revision was landed with ongoing or failed builds.Aug 24 2021, 1:00 AM
This revision was automatically updated to reflect the committed changes.
ikelarev added inline comments.
openmp/libomptarget/plugins/amdgpu/impl/data.cpp
24

Debug build is broken here:

data.cpp:24:41: error: use of undeclared identifier 'DeviceId'

DEBUG_PRINT("Malloced [CPU %d] %p\n", DeviceId, *ptr);
                                      ^
openmp/libomptarget/plugins/amdgpu/impl/data.cpp
24

Thanks for reporting! Simple fix at ba8547775b0c83ea. Something of a reminder that the files under impl are still using a different debug print mechanism to rtl.cpp.

@pushpinder we should delete the DEBUG_PRINT statements where reasonable and convert the survivors to the DP() macro.

openmp/libomptarget/plugins/amdgpu/impl/machine.h