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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
