[llvm] [mlir] [Flang][MLIR][OpenMP] Fix Target Data if (present(...)) causing LLVM-IR branching error (PR #123771)

[via llvm-commits]

https://github.com/agozillon created https://github.com/llvm/llvm-project/pull/123771

Currently if we generate code for the below target data map that uses an optional mapping:

       !$omp target data if(present(a)) map(alloc:a)
            do i = 1, 10
                a(i) = i
            end do
       !$omp end target data

We yield an LLVM-IR error as the branch for the else path is not generated. This occurs because we enter the NoDupPriv path of the call back function when generating the else branch, however, the emitBranch function needs to be set to a block for it to functionally generate and link in a follow up branch. The NoDupPriv path currently doesn't do this, while it's not supposed to generate anything (as far as I am aware) we still need to at least set the builders placement back so that it emits the appropriate follow up branch. This avoids the missing terminator LLVM-IR verification error by correctly generating the follow up branch.

>From e8016d3a2f9c9fa4e4256302718a545f44de509d Mon Sep 17 00:00:00 2001
From: agozillon <Andrew.Gozillon at amd.com>
Date: Tue, 21 Jan 2025 09:37:54 -0600
Subject: [PATCH] [Flang][MLIR][OpenMP] Fix Target Data if (present(...))
 causing LLVM-IR branching error

Currently if we generate code for the below target data map that uses an optional mapping:

       !$omp target data if(present(a)) map(alloc:a)
            do i = 1, 10
                a(i) = i
            end do
       !$omp end target data

We yield an LLVM-IR error as the branch for the else path is not generated. This occurs because we enter the NoDupPriv path of the call back function when
generating the else branch, however, the emitBranch function needs to be
set to a block for it to functionally generate and link in a follow up branch.
The NoDupPriv path currently doesn't do this, while it's not supposed to
generate anything (as far as I am aware) we still need to at least set the
builders placement back so that it emits the appropriate follow up branch.
This avoids the missing terminator LLVM-IR verification error by correctly
generating the follow up branch.
---
 .../OpenMP/OpenMPToLLVMIRTranslation.cpp      |  3 ++
 .../fortran/target-data-map-if-present.f90    | 29 +++++++++++++++++++
 2 files changed, 32 insertions(+)
 create mode 100644 offload/test/offloading/fortran/target-data-map-if-present.f90

diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index e776722a818d7d..c3d87e25ab12d2 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -3727,6 +3727,9 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase &builder,
       }
       break;
     case BodyGenTy::DupNoPriv:
+      // We must always restoreIP regardless of doing anything the caller
+      // does not restore it, leading to incorrect (no) branch generation.
+      builder.restoreIP(codeGenIP);
       break;
     case BodyGenTy::NoPriv:
       // If device info is available then region has already been generated
diff --git a/offload/test/offloading/fortran/target-data-map-if-present.f90 b/offload/test/offloading/fortran/target-data-map-if-present.f90
new file mode 100644
index 00000000000000..c181573cd7a1c1
--- /dev/null
+++ b/offload/test/offloading/fortran/target-data-map-if-present.f90
@@ -0,0 +1,29 @@
+! Offloading test that tests that if(present(a)) compiles and executes without
+! causing any compilation errors, primarily a regression test that does not
+! yield interesting results.
+! REQUIRES: flang, amdgpu
+
+! RUN: %libomptarget-compile-fortran-run-and-check-generic
+module mod
+    implicit none
+contains
+    subroutine routine(a)
+        implicit none
+        real, dimension(:), optional :: a
+        integer :: i
+       !$omp target data if(present(a)) map(alloc:a)
+            do i = 1, 10
+                a(i) = i
+            end do
+       !$omp end target data
+    end subroutine routine
+end module mod
+
+program main
+    use mod
+    real :: a(10)
+    call routine(a)
+    print *, a
+end program main
+
+! CHECK: 1. 2. 3. 4. 5. 6. 7. 8. 9. 10.