[Mlir-commits] [llvm] [mlir] [mlir][openmp] - Fix crash in OpenMPIRBuilder when converting to LLVMIR (PR #84611)

[Sergio Afonso]

https://github.com/skatrak commented:

Thanks for looking into this Pranav. It looks related to this downstream patch by @DominikAdamski: https://github.com/ROCm/llvm-project/pull/40.

It's yet another manifestation of the issues we have due to having to keep host code when compiling for the device just so that we're able to lower target regions inside. When we had the early outlining pass we didn't have this problem, because only device code remained by the time we lowered to LLVM IR. Now we're lowering all of the host code that exists in functions with an `omp.target` inside to then remove it later, which causes problems like what you're fixing here.

I think the long term solution for this is to actually remove host code from device compilation early (re-introducing some sort of early outlining, or at least somehow removing host operations in a pass while keeping target regions untouched). It seems difficult to selectively detect and skip lowering for these operations in advance in all cases, like Dominik's PR tries to do. And fixing the IR after lowering, like this patch does, is very difficult to get right for all situations, since lowering host operations might result in the creation of some OMPIRBuilder state used after the function has been deleted and this will change over time.

So I think this PR looks good as a short term solution to the issue, but we need to figure out a better plan long term. I'd like to hear from others' opinions about this more general issue. I don't oppose merging this PR, but I'll leave the approval to someone else.

https://github.com/llvm/llvm-project/pull/84611