[libc-commits] [PATCH] D139839: [libc] Add a loader utility for AMDHSA architectures for testing

Mon Dec 12 08:34:01 PST 2022

jhuber6 added a comment.

In D139839#3988765 <https://reviews.llvm.org/D139839#3988765>, @jdoerfert wrote:

> I don't think this is what we should do right now. We should have reused the existing offload runtime with the two simple overlay headers, effectively what we used here: https://tianshilei.me/wp-content/uploads/2022/09/llvm-hpc-22.pdf
> We would get support for all backends without having the 3rd! HSA driver in tree. Someone will start to improve the driver, we add a 3rd CUDA one, we start to build APIs, we end up with the plugin interface all over again.
> The right choices is to use it and to decouple it rather than to duplicate it.

I don't think it's a good idea to have an external dependency on the `openmp` runtime for the purpose of testing this project. The work here is the same concept as the direct GPU compilation work, just with the working parts separated out into a standalone application with minimal dependencies. This does duplicate HSA code, but I think the existence of these utilities is useful in its own right. If we want to write a `libc` on the GPU it's very useful to have a utility in-source that lets us run a generic source file on the GPU.

> On the "loader" question: Why do we need a loader executable in the first place? Why not patch the original up so we can just execute that one? Lot's of benefits (self contained, can have fallback path and multiple archs, ...)

I don't think supporting multiple architectures or having fallback is useful in this scenario, we just want to see if the source runs on the user's chosen GPU. If not then that's a failure. As for it being standalone, I think it just makes ti easier to use as a utility. We could change it to load from some internal pointer and recompile it for each test if that is worthwhile.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139839/new/

https://reviews.llvm.org/D139839