jhuber6 added a comment. In D128914#3642869 <https://reviews.llvm.org/D128914#3642869>, @yaxunl wrote:
> 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. I see, this wrapping will only happen in RDC-mode so it's probably safe to ignore here? When I support non-RDC mode in the new driver it will most likely rely on the old code generation. Although it's entirely feasible to make RDC-mode the default. There's no runtime overhead when using LTO. 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
