[PATCH] D128914: [HIP] Add support for handling HIP in the linker wrapper

Yaxun Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 11 10:25:02 PDT 2022


yaxunl added a comment.

In D128914#3642567 <https://reviews.llvm.org/D128914#3642567>, @jhuber6 wrote:

> In D128914#3642558 <https://reviews.llvm.org/D128914#3642558>, @JonChesterfield wrote:
>
>> Code looks good to me. It's hard to be sure whether it works without running a bunch of hip test cases through it, have you already done so? If it doesn't work out of the box it should be close enough to fix up post commit, e.g. when trying to move hip over to this by default.
>
> Thanks for the review, I ran a couple mini-apps with HIP versions (XSBench, RSBench, SU3Bench) using this method and they passed without issue. The only thing I was unsure about what whether or not the handle needed to be checked for null, because my testing suggested it's unnecessary. I was hoping one of the HIP developers would let me know. We can think about making this the default approach when I make the new driver work for `non-rdc` mode compilations.

There is only one fatbin for -fgpu-rdc mode but the fatbin unregister function is called multiple times in each TU. HIP runtime expects each fatbin is unregistered only once. The old embedding scheme introduced a weak symbol to track whether the fabin has been unregistered and to make sure it is only unregistered once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128914



More information about the cfe-commits mailing list