This is an archive of the discontinued LLVM Phabricator instance.

[libomptarget][nfc] Accept callable for hsa iterate_symbols
ClosedPublic

Authored by JonChesterfield on May 24 2021, 8:43 AM.

Details

Summary

[libomptarget][nfc] Accept callable for hsa iterate_symbols
Candidate refactor to simplify D102692

Diff Detail

Event Timeline

JonChesterfield requested review of this revision.May 24 2021, 8:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2021, 8:43 AM
  • drop dead executable argument
openmp/libomptarget/plugins/amdgpu/impl/system.cpp
1013

This change means populate_InfoTables can have a different type from the HSA callback, e.g. could take pointers to infotables directly

1243

...because there can now be a capturing lambda to do the impedance mismatch

pdhaliwal accepted this revision.May 24 2021, 11:33 PM

Looks good. My opinion is that we should keep these wrappers in a separate file, but that could be a later patch.

This revision is now accepted and ready to land.May 24 2021, 11:33 PM
This revision was landed with ongoing or failed builds.May 25 2021, 1:29 AM
This revision was automatically updated to reflect the committed changes.

Looks good. My opinion is that we should keep these wrappers in a separate file, but that could be a later patch.

Agreed, got stuck on picking a name and decided it was more important to get the diff up quickly.

There are a few things being considered for hsa:

  • easier API via C++ wrapper
  • wrapping calls in optional profiling collection
  • dlopen hsa instead of link, for building llvm on systems without hsa installed

They're all somewhat related. I haven't put enough thought into the design space yet.

foad removed a subscriber: foad.May 25 2021, 5:19 AM